security: bound Content-Length parsing in viewer server#875
Open
Sunil56224972 wants to merge 1 commit into
Open
security: bound Content-Length parsing in viewer server#875Sunil56224972 wants to merge 1 commit into
Sunil56224972 wants to merge 1 commit into
Conversation
The viewer's _read_body() method reads Content-Length bytes from the request without any upper bound. A client can send a crafted Content-Length: 999999999999 header and force the server to attempt allocating an enormous buffer, causing an out-of-memory denial of service against the viewer process. Additionally, a non-integer Content-Length value (e.g. 'abc' or '12,34') causes int() to raise ValueError. The outer handler catches it as a generic 500, but the real issue is a malformed request that should be silently dropped. Fixes: 1. Add _MAX_BODY_BYTES (2 MiB) cap -- any Content-Length above this limit causes _read_body to return an empty dict (same as a missing body), so the endpoint returns its normal validation error. 2. Wrap the int() conversion in try/except (ValueError, OverflowError) so malformed headers degrade gracefully instead of producing a 500 with a full stack trace in the log.
Contributor
Greptile SummaryThe PR hardens viewer request-body parsing.
Confidence Score: 5/5The PR appears safe to merge with no actionable issues identified. The viewer now rejects invalid or excessive declared body lengths before reading them, while valid request bodies continue through the existing JSON parsing path. Important Files Changed
Reviews (1): Last reviewed commit: "security: bound Content-Length parsing i..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The viewer server's
_read_body()method has two issues withContent-Lengthhandling:1. Unbounded memory allocation (OOM DoS) — Medium Severity
A client can send
Content-Length: 999999999999and force the server to attempt allocating an enormous buffer viaself.rfile.read(length), causing an out-of-memory denial of service against the viewer process.Fix: Added
_MAX_BODY_BYTES = 2 * 1024 * 1024(2 MiB) cap. AnyContent-Lengthabove this limit causes_read_body()to return an empty dict, so the endpoint returns its normal validation error.2. Unhandled ValueError on malformed Content-Length — Low Severity
int(self.headers.get('Content-Length') or 0)raisesValueErrorif the header contains a non-integer value (e.g.'abc'or'12,34'). The outerexcept Exceptioncatches it but produces a 500 + full stack trace in logs for what should be a malformed request.Fix: Wrapped the
int()conversion intry/except (ValueError, OverflowError)so malformed headers degrade gracefully.Files changed
strix/viewer/server.pyTesting