miguelgrinberg / Flask-HTTPAuth

Simple extension that provides Basic, Digest and Token HTTP authentication for Flask routes
MIT License
1.27k stars 228 forks source link

Fix to Clear TCP Buffer on Unauthorized Large File Uploads in Swagger-UI (#138) #167

Closed Nantero1 closed 1 week ago

Nantero1 commented 1 week ago

This PR addresses and improves the security issue originally raised in #138, where unauthorized large file uploads caused the backend to process the request, leading to potential DoS attacks. At the time, the request data was being read despite authentication failure, filling the disk or memory with large temporary files. We removed any data processing back then on unauthorized requests.

Upon further investigation and many years, we now have a deeper understanding of the previous behavior and why it was once there. The primary reason for initially reading the request data was not to process or store the uploaded form data, but to avoid a "failed to fetch" error reported by Swagger-UI due to incomplete request processing. When form data is uploaded, the backend must receive the data entirely, even if the request is unauthorized, or else the client fails to receive the backend response properly (Failed to Fetch error).

Proposed Fix:

Instead of parsing and processing the form data, the TCP receive buffer is cleared by reading the incoming stream in chunks of 64KB, discarding the data. This approach solves the "failed to fetch" error while preventing unauthorized form uploads from being processed unnecessarily.

References:

miguelgrinberg commented 1 week ago

I really prefer to not implement this hack again. This isn't something that Flask-HTTPAuth should be concerned about at its level in the stack. Either your web server (Gunicorn, uWSGI), or your reverse proxy (nginx, Apache) should take care of this low level task. The issue is not in this package and I think it was a mistake to allow the initial contribution.

Nantero1 commented 1 week ago

You're right, I don't like this hack either. I just came across it again after several years and finally understood the purpose behind the original contribution, so I wanted to share it with you. In case someone encounters "failed to fetch" errors in the future when uploading large form data files, they might find this conversation useful.

I've updated the contributed code to avoid using the internal function.

That said, I'll leave it here. While I don't particularly like this hack, it might be helpful for some people. I'm not envious of you having to make a decision 😁

We ourselves do not rely on this 'hack' since we override our authentication methods. I have already taken emptying the buffer into account there.

miguelgrinberg commented 1 week ago

I'm closing per discussion above. Anybody needing to do this can add the code in this PR to their Flask-HTTPAuth error handler.