July 21, 2026

Secure File Upload System

I Built a “Secure” File Upload System. It Wasn’t Secure Until I Tried to Break It Myself.

By Ericmpta

7 min read

I Built a "Secure" File Upload System. It Wasn't Secure Until I Tried to Break It Myself.

What seven phases of shipping bugs into my own security code taught me about the gap between "the code looks right" and "the code is right."

I spent a week-8 stretch of my security learning track building a Django file upload system. The brief was simple: build something that handles uploads safely-validate files, store them securely, gate downloads behind auth, log what happens, rate-limit abuse.

I could have written this in an afternoon if the goal was "code that compiles." It took a lot longer than that, because I made a rule for myself partway through: nothing counts as done until I've actually tried to break it and watched it hold.

That rule surfaced more bugs in my own "secure" code than any code review would have. Here's what happened.

Full code: github.com/Ericmuturimwangi/Django-file-upload-service

Extension checks are theater

The first version of the upload validator checked one thing: does the filename end in .jpg, .png, or .pdf? That's it. That's the whole check.

I renamed a PHP shell to shell.php.jpg and uploaded it. Accepted, every time. The extension check never looked at what was actually in the file — it just read the string after the last dot.

The fix was magic-byte sniffing with python-magic, which reads the actual file signature-the real bytes at the start of the file-rather than trusting anything the client claims. Now a PHP payload gets sniffed as text/x-php regardless of what extension you slap on it, and it gets rejected before it ever touches disk.

python

def validate_content_matches_extension(uploaded_file, ext):
    sniffed = sniff_mime(uploaded_file)
    if sniffed not in ALLOWED_TYPES:
        return f"Content type '{sniffed}' not permitted.", None
    if ext not in ALLOWED_TYPES[sniffed]:
        return "File extension does not match actual content.", None
    return None, sniffed

Lesson one, early and cheap: never validate a file based on anything the client tells you about itself. Extension, Content-Type header, filename-all of it is a claim, not a fact. The only fact is the actual bytes.

The rejection that didn't actually stop anything

Once extension checking felt too easy to bypass, I added a size limit. Reject anything over 5MB. Simple, right?

Except-and this took a specific, deliberate test to actually see -Django doesn't wait for my code to check file size. Depending on how big the upload is, Django's own upload handlers may already have streamed the entire file to a temp directory on disk before my view function even starts running. My validate_size check was real. It did reject oversized files. But it was cleaning up after a disk write that had already happened, not preventing one.

That's a genuinely different security property, and the difference matters: an attacker could still exhaust disk space by uploading huge files over and over, getting a clean 400 every time, while quietly filling up storage in the background.

The actual fix lives at a layer above the view function entirely — middleware that checks Content-Length before Django's file handling kicks in:

python

class MaxUploadSizeMiddleware:
    def __call__(self, request):
        if request.method == "POST":
            content_length = request.META.get("CONTENT_LENGTH")
            if content_length and int(content_length) > MAX_UPLOAD_BYTES:
                return JsonResponse({"error": "Request too large."}, status=413)
        return self.get_response(request)

I proved this actually worked by sending a 10MB file against a 5MB middleware cap and watching two things: a 413 response, fast, and-this is the part that mattered -an empty private_uploads/ directory afterward. Not "the response looked right." The directory was actually, verifiably empty.

Even this isn't the full story. Content-Length is a header the client sends-a well-behaved client sends it honestly, but nothing stops a malicious one from lying or omitting it. The real, unconditional boundary belongs to whatever reverse proxy sits in front of Django in production (client_max_body_size in Nginx), not to anything in my codebase. I wrote that down as a known limitation instead of pretending my middleware was the whole answer.

The bug that survived three "fixes"

This is the one that actually humbled me.

Polyglot files are the classic bypass for image validation: take a real, valid JPEG, and just… append something else after it. A JPEG decoder stops reading at the end-of-image marker. Most naive validators do too. So a file can pass every check that only looks at the beginning and still smuggle a payload at the end.

I built a test file-a real photo with <?php system($_GET['cmd']); ?> glued onto the end-and confirmed it sailed straight through my magic-byte validation, because the header was a perfectly legitimate JPEG. Then I grep'd the stored file on disk and found the PHP string sitting right there, in a file my own system had labeled "accepted."

The fix -the real fix, not just a check -is re-encoding. Fully decode the image into raw pixel data, then build a brand-new file from scratch using only those pixels. Anything that wasn't part of the actual image-trailing bytes, embedded scripts, EXIF metadata-never survives the trip, because the new file has no memory of the old file's bytes at all:

python

def reencode_image(file_obj, verified_mime):
    img = Image.open(file_obj)
    img.load()  # force full pixel decode
    clean = Image.new(img.mode, img.size)  # brand new, no file association
    clean.putdata(list(img.getdata()))  # only the pixels survive
    out = io.BytesIO()
    clean.save(out, format=FORMAT_MAP[verified_mime])
    return out

I wrote this function correctly on the first try. And the payload still made it to disk anyway -twice-because two turns later, the actual call site that saved the file was still passing the original upload into storage, not the re-encoded version. The fix existed. It just wasn't wired in.

That's the pattern I want to flag hardest: code that is correct in isolation tells you nothing about whether it's actually connected. I found the same failure mode again a few phases later -a token-signing function written correctly, imported nowhere, crashing the endpoint the moment it was hit for real. Reading code and running code are different verbs, and only one of them tells you the truth.

Removing a security check I'd added myself

Somewhere around phase 3, hunting down an early CSRF error while trying to get a curl test to pass, I slapped @csrf_exempt on the upload endpoint. It made the error go away. I moved on.

It sat there, unexamined, for several phases -genuinely live, shippable, "working" code with a real hole in it.

What eventually surfaced it wasn't a code review. It was being forced to explain, out loud, what CSRF actually protects against: a malicious site tricking a logged-in victim's browser into firing off a request using their session, without their knowledge or consent. Once I said that sentence and then asked "does that risk apply to file uploads specifically"-the answer was obviously yes. A hostile page could silently upload garbage using someone else's account, fill their storage, or worse. There was no good reason for that exemption. It existed purely because it made an early test pass, and nobody had gone back to ask if it was correct, only whether it was convenient.

I removed it. Every upload test after that had to go through the real CSRF dance -fetch a token, send it, verify the session-same as everything else.

The lesson generalizes past this one endpoint: a security control you can't explain the threat model for is a control you'll eventually remove without noticing you've reopened a hole. "It made the error go away" and "it's actually safe" are not the same sentence, and it's worth never letting them blur together.

Deletion semantics are a security decision

Small, easy to skate past: when I built the audit log model, I had to pick what happens to a log entry when the user or file it references gets deleted. Django gives you CASCADE (delete the log row too) or SET_NULL (keep the row, just blank the reference).

I almost picked CASCADE on autopilot, because it "keeps things tidy." Then I actually thought through the scenario an audit log exists for: someone gets caught doing something malicious, and their account gets deleted-either by them, trying to cover their tracks, or by an admin doing routine cleanup after the fact. Under CASCADE, that deletion also erases the exact evidence of what they did. The log disappears at precisely the moment it matters most.

SET_NULL keeps the row. The action is still on record-"someone, no longer identifiable, did X to file Y at time Z"-even after the account is gone.

An ORM field option that looks like a modeling detail turned out to encode a real security guarantee. That's a pattern worth watching for generally: the "boring" config choices are sometimes the actual decision, dressed up as plumbing.

Predicting before testing changed what testing was for

Partway through this project I started forcing myself to state a prediction before running any test-what status code do I expect, what should the file on disk look like, why. This felt like busywork at first. It wasn't.

Without a stated prediction, every test result is just a fact with no context-you can't tell "expected" from "surprising," so nothing teaches you anything. Once I started predicting first, wrong predictions became the most valuable thing that happened all week. I predicted img.load() was the reason a polyglot payload got stripped; it turned out getdata() was already forcing that decode, and the explicit .load() call had been inert the whole time. I predicted an anonymous request would get a 403; it got a 302, which taught me @login_required redirects rather than denies-a decorator-ordering fact I'd never actually had to think about before.

Neither of those corrections would have landed if I'd just run the command and moved on to whatever came back.

What's still open, on purpose

I'm not going to pretend this system is finished, because it isn't, and pretending otherwise is its own kind of dishonesty:

  • Download tokens aren't single-use. A valid token works for anyone holding it until its 5-minute window runs out, even after a successful download.
  • Rate limiting is per-user, which stops one account from hammering the system but does nothing against an attacker spinning up many accounts.
  • No malware scanning. Re-encoding defeats format-confusion and polyglot tricks; it does nothing against a payload built to exploit a bug in the image-decoding library itself.
  • No storage quota per user.

These aren't things I forgot. They're things I know about and chose to name explicitly rather than let someone else discover later.

The actual takeaway

None of the bugs in this post were exotic. Every single one was something I'd have told you, if you'd asked me directly, "no, of course my code doesn't do that." And every single one was real, sitting in code that looked correct, until I built a specific adversarial test and watched it fail.

"I wrote the defense" and "the defense works" turned out to be two different claims, separated by exactly one real test -and the whole week was really just an exercise in refusing to let myself round the first claim up to the second.