Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAX_CONTENT_LENGHT not being respected with JSON payload #1200

Closed
ispmarin opened this issue Oct 9, 2014 · 9 comments
Closed

MAX_CONTENT_LENGHT not being respected with JSON payload #1200

ispmarin opened this issue Oct 9, 2014 · 9 comments

Comments

@ispmarin
Copy link

ispmarin commented Oct 9, 2014

Setting the MAX_CONTENT_LENGHT variable does nothing to limit a json payload larger than the set limit. We are using Flask Classy to write the post method and Flask to handle the requests.

<Config {'JSON_AS_ASCII': True, 'BASIC_AUTH_REALM': '', 'MONGO_UNIX_SOCKET': '/tmp/mongodb-27017.sock', 'REC_SYS_API_PORT': 5000, 'SQLALCHEMY_POOL_RECYCLE': None, 'MONGO_PORT': 27017, 'PROJECT_ROOT': '/home/project', 'SQLALCHEMY_POOL_TIMEOUT': None, 'SQLALCHEMY_RECORD_QUERIES': None, 'SESSION_COOKIE_DOMAIN': None, 'SESSION_COOKIE_NAME': 'session', 'DB_NAME': 'test_tracks', 'TRACK_ACTIONS': ['apply'], 'SQLALCHEMY_NATIVE_UNICODE': None, 'MAX_CONTENT_LENGTH': 65536, 'PERMANENT_SESSION_LIFETIME': datetime.timedelta(31), 'SQLALCHEMY_POOL_SIZE': None, 'SQLALCHEMY_MAX_OVERFLOW': None, 'TRAP_HTTP_EXCEPTIONS': False, 'BASIC_AUTH_FORCE': False, 'PRESERVE_CONTEXT_ON_EXCEPTION': None, 'SQLALCHEMY_ECHO': False, 'SESSION_COOKIE_PATH': None, 'LOGGER_NAME': 'rtr', 'MONGO_USE_SOCKET': True, 'SECRET_KEY': None, 'APP_NAME': 'trkc', 'SERVER_NAME': None, 'PREFERRED_URL_SCHEME': 'http', 'TESTING': False, 'MONGODB_SETTINGS': {'DB': 'test_tracks'}, 'USE_X_SENDFILE': False, 'SQLALCHEMY_DATABASE_URI': 'sqlite:////tmp/test.db', 'SESSION_COOKIE_SECURE': False, 'SEND_FILE_MAX_AGE_DEFAULT': 43200, 'SQLALCHEMY_BINDS': {'logsite': 'sqlite:////tmp/test_logsite.db'}, 'DEBUG': False, 'SQLALCHEMY_COMMIT_ON_TEARDOWN': False, 'APPLICATION_ROOT': None, 'JSONIFY_PRETTYPRINT_REGULAR': True, 'PROPAGATE_EXCEPTIONS': None, 'TRAP_BAD_REQUEST_ERRORS': False, 'JSON_SORT_KEYS': True, 'SESSION_COOKIE_HTTPONLY': True, 'MONGO_HOST': 'localhost'}>

Cheers

@untitaker
Copy link
Contributor

Flask/Werkzeug only validate the content length when accessing the relevant request attributes.

@ispmarin
Copy link
Author

ispmarin commented Oct 9, 2014

Could you please elaborate which relevant request attributes are being checked? The info in the documetation mentions "If set to a value in bytes, Flask will reject incoming requests with a content length greater than this by returning a 413 status code.", so I assumed that if my payload on a POST is larger than MAX_CONTENT_LENGTH it would be blocked with 413. Thanks for your patience!

@untitaker
Copy link
Contributor

Flask lazily parses and validates incoming request data as you access e.g. request.form. See https://github.com/mitsuhiko/flask/blob/master/tests/test_basic.py#L1143, if you remove the access to flask.request.form['myfile'] inside the view functions, the testcase will fail.

@untitaker
Copy link
Contributor

And the same holds true for basically every request attribute that consumes the request body.

@untitaker
Copy link
Contributor

@ispmarin Is this answer sufficient for you and can i close the issue?

@ispmarin
Copy link
Author

Got it, gonna investigate more the effects. Thanks

@karlud
Copy link

karlud commented Jun 23, 2017

Pardon me for reopening this. There still seems to be a possible DoS safety issue here. If an endpoint calls request.get_json() before accessing any of the fields that trigger MAX_CONTENT_LENGTH checking, then it will read all the POSTed data into memory.

Here's an example server and client that demonstrate this, using the resource module to inspect memory usage:

Server:

#!/usr/bin/env python3

from flask import Flask, request
import resource

app = Flask(__name__)
app.config['MAX_CONTENT_LENGTH'] = 1000

@app.route('/mem')
def mem():
  return str(resource.getrusage(resource.RUSAGE_SELF).ru_maxrss)

@app.route('/unsafe', methods=['POST'])
def unsafe():
  j = request.get_json()
  return 'ok'

@app.route('/safe', methods=['POST'])
def safe():
  j = request.data
  j = request.get_json()
  return 'ok'

if __name__ == '__main__':
  app.run()

Client:

#!/usr/bin/env python3

import requests

r = requests.get("http://localhost:5000/mem")
print("Starting memory usage:", r.text)

r = requests.post("http://localhost:5000/safe", json=["small req"])
print(r.text)

r = requests.get("http://localhost:5000/mem")
print("After small request to /safe:", r.text)

r = requests.post("http://localhost:5000/unsafe", json=["small req"])
print(r.text)

r = requests.get("http://localhost:5000/mem")
print("After small request to /unsafe:", r.text)

try:
  r = requests.post("http://localhost:5000/safe", json=["large req"*59999999])
  print(r.text)
except Exception as e:
  print("fail")

r = requests.get("http://localhost:5000/mem")
print("After large request to /safe:", r.text)

r = requests.post("http://localhost:5000/unsafe", json=["large req"*59999999])
print(r.text)

r = requests.get("http://localhost:5000/mem")
print("After large request to /unsafe:", r.text)

Example run:

$ python3 testclient.py
Starting memory usage: 20631552
ok
After small request to /safe: 20709376
ok
After small request to /unsafe: 20713472
fail
After large request to /safe: 20992000
ok
After large request to /unsafe: 1389424640

The /safe endpoint, which peeks at request.data before running request.get_json(), successfully avoids allocating a gigantic amount of memory for the gigantic incoming POST body. However, the /unsafe endpoint, which doesn't, allocates >1GB memory for that POST body.

In other words, it sure seems like JSON servers that want to protect themselves from allocating arbitrary amounts of memory in response to a large POST must do something to cause MAX_CONTENT_LENGTH checking before using request.get_json().

@davidism
Copy link
Member

Closed by pallets/werkzeug#1126.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
@davidism
Copy link
Member

Fixed by pallets/werkzeug#2620

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants