Fix to Clear TCP Buffer on Unauthorized Large File Uploads in Swagger-UI (#138) #167
Conversation
src/flask_httpauth.py
Outdated
| @@ -14,6 +14,7 @@ | |||
| from random import Random, SystemRandom | |||
| from flask import request, make_response, session, g, Response, current_app | |||
| from werkzeug.datastructures import Authorization | |||
| from werkzeug.formparser import _chunk_iter | |||
There was a problem hiding this comment.
The _ prefix in this function name indicates that it is private and is not supposed to be used externally. I have been burned many times by using private functions from Werkzeug that are routinely renamed or relocated to different modules, so I don't consider this an acceptable solution.
|
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. |
…arge form uploads Instead of parsing the entire request data, the request buffer is now cleared by reading the data into /dev/null. This prevents the backend from fully processing unauthorized form uploads, while fixing the "failed to fetch" issue on large uploads through Swagger-UI.
Nantero1
force-pushed
the
bugfix/clear_tcp_bugger
branch
from
f64e3e7
to
8646ae9
Compare
|
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. |
|
I'm closing per discussion above. Anybody needing to do this can add the code in this PR to their Flask-HTTPAuth error handler. |
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: