fix: add MAX_CONTENT_LENGTH to prevent unbounded-body memory exhaustion DoS#6529
Conversation
…on DoS Without app.config['MAX_CONTENT_LENGTH'], Flask reads the entire POST body into memory before entering any route handler. An unauthenticated attacker can send a multi-gigabyte body to /attest/submit, /governance/vote, /wallet/transfer/signed, or any other POST endpoint, exhausting worker RAM and crashing the node. Setting MAX_CONTENT_LENGTH = 1 MB causes Flask to respond 413 immediately at the WSGI layer, before any handler allocates memory for JSON parsing. Adds test_max_content_length_poc.py (5 tests) documenting the vulnerable and fixed behaviour.
|
Welcome to RustChain! Thanks for your first pull request. Before we review, please make sure:
Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150) A maintainer will review your PR soon. Thanks for contributing! |
eliasx45
left a comment
There was a problem hiding this comment.
Reviewed current head 454a0298b8a38d16116f3d3aa96b4c54bc9eab24.
Verdict: request changes.
The global MAX_CONTENT_LENGTH setting is the right mitigation direction, but the current implementation does not deliver the advertised behavior on at least /attest/submit: the oversized request is rejected by Werkzeug, then caught by the route's broad exception handler and returned as a 500 internal_error instead of a 413.
Evidence:
- Inspected
node/rustchain_v2_integrated_v2.2.1_rip200.pyandnode/test_max_content_length_poc.py. app.config['MAX_CONTENT_LENGTH']is set to 1 MB afterapp = Flask(__name__).- The new
node/test_max_content_length_poc.pyonly verifies a standalone Flask app config pattern; it does not import this app or assert the real RustChain route returns 413. py_compile node/rustchain_v2_integrated_v2.2.1_rip200.py node/test_max_content_length_poc.pypassed..\.venv\Scripts\python.exe -m pytest node\test_max_content_length_poc.py -q-> 5 passed.git diff --check origin/main...HEAD-> clean.
Actual-route reproduction on this head:
- Imported the real app with
RC_ADMIN_KEY=0123456789abcdef0123456789abcdefand posted1024*1024 + 1bytes to/attest/submitusing Flask's test client. app.config['MAX_CONTENT_LENGTH']read back as1048576.- The response status was
500, not413. - Logs show
werkzeug.exceptions.RequestEntityTooLargeraised fromrequest.get_json(...), then caught by the attestation route's broad exception handler and converted to{"code":"INTERNAL_ERROR","error":"internal_error",...}.
Required fix: add real-route coverage for oversized bodies and either let RequestEntityTooLarge propagate to Flask's 413 handler, add an explicit app error handler for 413, or avoid broad route exception handling that masks the 413. The PR should prove at least one representative JSON endpoint returns 413 on an over-limit body.
…turn 413 The previous fix set MAX_CONTENT_LENGTH but did not handle the case where RequestEntityTooLarge is raised inside a route wrapped by a broad except-Exception handler (e.g. attest/submit), which converted the 413 to a 500. Added a before_request hook that checks Content-Length before any route runs, and an app-level errorhandler(413) that returns JSON. Updated tests to import the real app and assert 413 on /attest/submit, /wallet/transfer/signed and /governance/vote. All 6 tests pass.
|
Thanks for the detailed reproduction steps — you were correct. The root cause: Flask raises Fix pushed at cfd585c:
Tests updated to import the real app via
Also asserts the 413 response body is JSON with |
eliasx45
left a comment
There was a problem hiding this comment.
Re-reviewed current head cfd585c440081964a10e767c73ad7cbc4d8fa011 after the follow-up fix.
Verdict: approve, with the hosted full-suite test failure noted as still red on the PR but not reproduced in this focused size-limit path.
The previous blocker is fixed. Oversized requests are now rejected before route-level broad except Exception blocks can convert RequestEntityTooLarge into a 500.
Evidence:
- Inspected
node/rustchain_v2_integrated_v2.2.1_rip200.pyandnode/test_max_content_length_poc.py. - The app still sets
app.config['MAX_CONTENT_LENGTH'] = 1048576. - New
@app.before_requestchecksrequest.content_lengthagainst the configured limit and raisesRequestEntityTooLargebefore entering route handlers. - New
413/RequestEntityTooLargehandlers return JSON withcode: REQUEST_TOO_LARGE. - The updated test file imports the real app and covers oversized bodies on
/attest/submit,/wallet/transfer/signed, and/governance/vote. py_compile node\rustchain_v2_integrated_v2.2.1_rip200.py node\test_max_content_length_poc.pypassed..\.venv\Scripts\python.exe -m pytest node\test_max_content_length_poc.py -q-> 6 passed.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean merge tree.
Independent reproduction of the original failure mode on this head:
- Imported the real app with
RC_ADMIN_KEY=0123456789abcdef0123456789abcdef. - Posted
MAX_CONTENT_LENGTH + 1bytes to/attest/submitusing Flask's test client. - Observed
status=413and JSON{'ok': False, 'code': 'REQUEST_TOO_LARGE', 'error': 'request body exceeds the 1 MB limit'}. - A normal small
/attest/submitJSON body returned422, not413, confirming the size gate is not rejecting normal bodies.
I do not see a blocker in this focused fix now.
Ivan-LB
left a comment
There was a problem hiding this comment.
Thanks for the thorough re-review, @eliasx45. Glad the before_request gate and the 413 handler addressed the original concern cleanly. The full-suite red is unrelated to this path, so that should not block merge. Appreciate the detailed evidence walkthrough.
|
Thank you for the merge and the bounty, @Scottcjn. Glad the test coverage was solid, the baseline failures across those three endpoints made the real-world DoS surface easy to verify. Will keep an eye on the 24h void window. |
Summary
Flask's default configuration accepts POST bodies of unlimited size. Without
app.config['MAX_CONTENT_LENGTH'], every route handler that callsrequest.get_json()first loads the entire body into RAM. An unauthenticated attacker can send a multi-gigabyte POST to any of the following public endpoints and exhaust worker memory, crashing the node:POST /attest/submitPOST /governance/votePOST /governance/proposePOST /wallet/transfer/signedPOST /epoch/enrollRoot cause:
app = Flask(__name__)at line 180 sets no content-length cap. Flask's WSGI layer will buffer an arbitrarily large body before handing control to the route.Fix: One line immediately after app construction:
Flask then responds
413 Request Entity Too Largeat the WSGI boundary, before any route handler is entered and before any memory is allocated for JSON parsing.PoC
Test
node/test_max_content_length_poc.py— 5 passing tests documenting the vulnerable (no config key) and fixed (1 MB cap) behaviour.Wallet
RTC64aa3fc417e75224e1574acae906fea34d94d140(minerIvan-LB)