Critical security issue when uploading files

[Nantero1]

I hope I'm wrong, but maybe I found a security issue 🔓

Line 166 is problematic:

Flask-HTTPAuth/src/flask_httpauth.py

Lines 164 to 166 in dc6de2d

	if not optional and status:
	# Clear TCP receive buffer of any pending data
	request.data

Even if the user is unauthorized, files send via a form will be processed. In case of large files, even local temp files will be created. This can be used for DoS attacks, by flooding the Flask service with large files, filling the temp drive and/or keeping the service threads occupied until the whole service becomes unresponsive 💣 .

Please run this example code, upload a large file and authenticate yourself with a wrong username/password combination. Watch the service process the large file (response takes some time, service is occupied by this task) or even debug and watch the temporary file being created on the disk, despite a wrong authentication was given 😢 .

"""
Flask-Restx is used for easier file upload, as it provides a Swagger-UI user interface, otherwise it is not needed

Navigate to http://127.0.0.1:5000/ and use the GUI to upload a large file
Place a debug-point in the last line `return self.cls(fields), self.cls(files)` of werkzeug.formparser.MultiPartParser
class and observe, how the temporary file is created and stored locally, despite no authorization was provided.
"""

from flask import Flask
from flask_httpauth import HTTPBasicAuth
from flask_restx import Api
from flask_restx import reqparse, Resource
from werkzeug.datastructures import FileStorage
from werkzeug.security import generate_password_hash, check_password_hash

app = Flask(__name__)
auth = HTTPBasicAuth()
api = Api(app)

# doesn't matter:
users = {
    "john": generate_password_hash("hello"),
    "susan": generate_password_hash("bye")
}

parser = reqparse.RequestParser()
parser.add_argument('some_file', location='files', type=FileStorage, required=True,
                    help="Please upload a large file (GBytes) and authenticate yourself with a wrong username")


@auth.verify_password
def verify_password(username, password):
    """
    This doesn't matter, call the service with a wrong user, send file will still be processed
    """
    if username in users and check_password_hash(users.get(username),
                                                 password):
        return username


@api.route('/upload')
class HelloWorld(Resource):
    @api.expect(parser)
    @auth.login_required
    def post(self):
        return {'hello': 'world'}


if __name__ == '__main__':
    app.run(debug=True, host='127.0.0.1')

I would suggest, NOT to clear the TCP receive buffer of any pending data, when the authentication is invalid.

For others: My workaround is to use flask.abort(401) immediately, after the username and password verification fails in the verify_password function.

[miguelgrinberg]

This is a change that was contributed many years ago (#38 and #39). I don't remember much about that bug report, but yeah, seems it would be best for this extension to not read the request, considering the possibility that resources (memory, disk space) have to be used. For any cases where this is a problem, the application and read the request itself at their own risk.

[miguelgrinberg]

Release 4.5.0 is out with this fix. Thanks!