Skip to content

HttpSignature::parse panics on a Signature-Input where ')' precedes '(', reachable pre-auth by any caller #348

Description

@beardthelion

HttpSignature::parse locates the component list with two independent searches and then slices
between them (crates/gitlawb-core/src/http_sig.rs:60-67):

let open = rest
    .find('(')
    .ok_or_else(|| Error::HttpSignature("missing '(' in Signature-Input".into()))?;
let close = rest
    .find(')')
    .ok_or_else(|| Error::HttpSignature("missing ')' in Signature-Input".into()))?;

let components_str = &rest[open + 1..close];

Nothing requires open < close. When ) appears first the range is reversed and the slice panics.
Note the function is otherwise careful: every other malformed shape returns a typed Error and the
caller turns it into a 400. This one input class walks straight past that.

Measured

The indexing above, extracted verbatim into a standalone binary (a yanked transitive dep blocks
building a scratch crate against gitlawb-core itself, so this is the five cited lines copied exactly
rather than a link against the crate):

reversed parens                -> PANIC
close-before-open w/ params    -> PANIC
well-formed (control)          -> Ok(("\"@method\"", ";keyid=\"k\""))
missing paren (control)        -> Err(missing '(')
bad prefix (control)           -> Err(must start with 'sig1=')

The three controls are the point: the panic is specific to the reversed-range condition, not a
blanket failure of malformed input.

Reachability

Pre-authentication, and the caller needs no credentials of any kind.

crates/gitlawb-node/src/auth/mod.rs:88 calls it inside require_signature, before any key is
resolved or checked, which is what makes this a parse of fully untrusted bytes. And
optional_signature (auth/mod.rs:251-257) delegates to require_signature whenever the request
merely carries signature headers:

let has_signature_headers = request.headers().contains_key("signature-input")
    || request.headers().contains_key("signature");
if has_signature_headers {
    return require_signature(request, next).await;
}

So the panic is reachable on routes that are otherwise anonymous, by sending:

Signature-Input: sig1=)(
Signature: sig1=:AAAA:

Blast radius, stated honestly

grep -rn "CatchPanic\|catch_unwind" crates/gitlawb-node/src/ finds only a test helper in
api/repos.rs, so there is no panic-catching layer on the router, and the release profile
(Cargo.toml:59-61) sets lto and strip but not panic = abort. So this unwinds and kills the
request task rather than the process: the caller gets a dropped connection instead of the 400 the
code intends, plus a logged backtrace per attempt.

That is why I am filing this medium rather than high. It is trivially scriptable and it is pre-auth,
but it costs one request per attempt and corrupts no state, which puts it alongside #317 (anonymous
malformed limit yielding a raw DB error) rather than alongside a resource-exhaustion issue. I have
NOT driven this end to end through a running node; the panic and the call chain are established by
execution and by reading respectively, and the "axum does not catch it" step is inferred from the
absence of a catch layer.

Fix direction

Require open < close before slicing, and return the existing Error::HttpSignature when it does
not hold, which keeps every malformed input on the one path the caller already handles. A regression
test should drive sig1=)( and assert a 400 rather than a panic, since a test that only asserts
is_err() would pass today by aborting the test thread.

Worth noting for whoever picks this up: PRs #306 and #261 both touch http_sig.rs and both add tests
around parse, but neither changes this indexing, so the panic survives both. Coordinating with them
is probably worth more than the fix itself, which is one comparison.

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:coregitlawb-core — identity, certs, encrypt, DID/UCANkind:securityVulnerability fix or hardeningsev:mediumDegraded but workaround existssubsystem:identityDID/UCAN, http-sig auth, push authorization

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions