fix: two remote-triggerable panics (DoS) from untrusted input - #98
Merged
Conversation
Several HTTP paths called HeaderValue::to_str().unwrap() on response headers: the response-context builder, the Location redirect handler, the Set-Cookie reader, the CSRF cookie reader, and the port scanner's HTTP banner grabber. to_str() rejects non-ASCII bytes, but RFC 7230 allows obs-text (0x80..=0xFF) in header values, so a single such byte from a malicious or misbehaving target server panics the process. Since the release profile sets panic = "abort", that panic aborts the whole scan, so a host you merely scan can take down the operator's legba. Decode the header bytes lossily instead of unwrapping.
parse_multiple_targets_atom captured each octet of an IPv4 range target as \d+ and then did parse::<u8>().unwrap(), so a target such as "256.0.0.0-1" (any octet > 255) panicked. Under the release profile's panic = "abort" this aborts the process, and the target parser is reachable from the unauthenticated REST/MCP API, so one request can take down the server. Parse the octets fallibly and return an error for out-of-range values. Adds a regression test.
Owner
|
LGTM thanks @gigioneggiando |
This was referenced Jul 5, 2026
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
2 participants
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.
Ciao Simone, as we discussed by email, here are the first fixes for the DoS issues from the private report. Two independent, self-contained, remote-triggerable panics, one commit each.
Both matter specifically because the release profile sets
panic = "abort", so any of these panics aborts the whole process instead of unwinding.1. Non-ASCII HTTP response headers (commit 1)
Several HTTP paths call
HeaderValue::to_str().unwrap()on response headers:plugins/http/mod.rs: the response-context builder (every header), theLocationredirect handler, and theSet-Cookiereaderplugins/http/csrf.rs: the CSRF cookie readerplugins/port_scanner/grabbers/http.rs: the HTTP banner grabberto_str()rejects non-ASCII bytes, but RFC 7230 allows obs-text (0x80..=0xFF) in header values, so a single such byte from a target server panics (aborts) the operator's legba: a host you merely scan can take you down. Fixed by decoding the bytes lossily (String::from_utf8_lossy), which is byte-identical in the normal ASCII case.2. Out-of-range IPv4 range octets (commit 2)
parse_multiple_targets_atomcaptures each octet of an IPv4 range target as\d+and then doesparse::<u8>().unwrap(), so a target like256.0.0.0-1(any octet > 255) panics. This parser is reachable from the unauthenticated REST/MCP API, so one request aborts the server. It now parses the octets fallibly and returns an error for out-of-range values; a regression test is included.No dependency changes and no behavior change beyond turning these panics into graceful handling. Happy to split this into two PRs or adjust anything. More of the report's fixes (server length-field allocation caps, read timeouts) will follow in separate PRs.