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

Critical security issue when uploading files #138

Closed Nantero1 closed 3 years ago

Nantero1 commented 3 years ago

I hope I'm wrong, but maybe I found a security issue πŸ”“

Line 166 is problematic: https://github.com/miguelgrinberg/Flask-HTTPAuth/blob/dc6de2dc9e1203e42d2763a0914e16ce96b74035/src/flask_httpauth.py#L164-L166

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 commented 3 years ago

This is a change that was contributed many years ago (https://github.com/miguelgrinberg/Flask-HTTPAuth/issues/38 and https://github.com/miguelgrinberg/Flask-HTTPAuth/issues/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 commented 3 years ago

Release 4.5.0 is out with this fix. Thanks!