harden(sip-parser): extract_uri must not panic on '>' before '<' (#114) - #118
Closed
ryanmurf wants to merge 1 commit into
Closed
harden(sip-parser): extract_uri must not panic on '>' before '<' (#114)#118ryanmurf wants to merge 1 commit into
ryanmurf wants to merge 1 commit into
Conversation
`extract_uri` computed `start = find('<')`, `end = find('>')` and sliced
`header_value[start+1..end]`. A SIP header whose value contains a '>' before
the URI's '<' — legal in a quoted display-name, e.g. `"a>b" <sip:carol@host>`
— makes `end < start`, a reversed byte range that panics. Reachable from the
request-handling path (dialog creation, registrar, ACL) on attacker-controlled
From/To/Contact headers; the panic aborts the SIP receive loop (DoS).
Search for '>' only in the tail after '<' (also more correct: it matches the
bracket that closes the URI, not an earlier one in the display-name).
Regression tests added; both panic-guards fail RED (panic at the slice) with
the fix reverted, while the well-formed guard passes in both states.
Fixes #114. (The Content-Length char-boundary panic originally bundled here,
#112, was fixed independently on master by #113/#108 — dropped from this PR.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XH6jhrG4ZCL8xfCktK4Zms
ryanmurf
force-pushed
the
hardening/proto-parser-panics
branch
from
July 17, 2026 00:49
c639870 to
85c5b9d
Compare
ryanmurf
changed the title
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
1 participant
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.
Fixes #114. Rebased onto master; the Content-Length char-boundary panic originally bundled here (#112) was fixed independently by #113/#108 and dropped. This PR is now just the distinct extract_uri bug.
Finding
extract_uri computed start=find('<'), end=find('>'), then header_value[start+1..end]. A SIP header value with '>' before the URI '<' — legal in a quoted display-name, e.g. "a>b" sip:carol@host — makes end < start, a reversed byte range that panics. Reachable from the request path (dialog creation, registrar, ACL) on attacker-controlled From/To/Contact headers; called inline from SipStack::run so the panic aborts the SIP receive loop (remote unauthenticated DoS).
Fix
Search for '>' only in the tail after '<' (also more correct: matches the bracket that closes the URI).
Tests + RED control
Three regression tests. With the fix reverted, the two panic-guards FAIL RED (panic at parser/mod.rs:867), the well-formed guard passes. With the fix: asterisk-sip lib green, full cargo test --workspace --exclude pjsip-shim green, clippy -D warnings clean. Toolchain 1.97.0.
Generated with Claude Code