Skip to content

check_created wraps on an extreme created timestamp: the freshness gate accepts one crafted value in release, panics in debug #358

Description

@beardthelion

crates/gitlawb-core/src/http_sig.rs:117-126:

pub fn check_created(&self) -> Result<()> {
    let now = Utc::now().timestamp();
    let skew = (now - self.created).abs();
    if skew > 300 {

created: i64 (:41) is parsed from the attacker-supplied created= parameter with no range clamp
(:90-94). Both the subtraction and the abs() can wrap.

Ran the matrix with the arithmetic copied verbatim, now = 1_755_000_000, built both ways:

created release (wrapping) debug (overflow-checks)
now (control) accepted accepted
now - 301 rejected, skew 301 rejected, skew 301
i64::MAX rejected rejected
i64::MIN rejected PANIC
now - 2^63 ACCEPTED PANIC

Two different bugs on one line. In release, now - created wraps to exactly i64::MIN, and
i64::MIN.abs() wraps back to i64::MIN, which is not > 300, so a true skew of 2^63 seconds reads
as fresh. In debug, the subtraction panics outright.

Production wraps rather than panics: the release profile (Cargo.toml:59-61) sets lto and strip
only, grep -rn "overflow-checks" across the tomls returns nothing, and shipped builds are
--release (Dockerfile:37,50, release.yml:448).

What it does not achieve, stated plainly

Passing check_created does not skip signature verification, and created is inside the signing
string, so the attacker has to be the signer. A legitimate signer gains nothing by wrapping the gate,
since they could simply send created=now. It does not extend replay life either: a captured
signature carries a fixed created, and the wrap condition is only met when now == created + 2^63.

So in release this is a broken invariant (a freshness gate that can be made to accept a value it
exists to reject) with no attacker benefit I can demonstrate. The panic is genuinely pre-auth and
permissionless, but only reaches cargo test, cargo run, and CI, never a shipped node.

Filing it at low on that basis rather than dressing it up.

Reachability

check_created is called from crates/gitlawb-node/src/auth/mod.rs:103 and
crates/git-remote-gitlawb/src/main.rs:1160. In the node it runs after HttpSignature::parse and
before missing_components, the alg check, DID resolution, and signature verification, so it needs
two headers and no registration.

Same function family and the same pre-auth entry point as #348, one step later in the middleware.
Different defect: #348 is a slice panic that fires in release too, which is why it rates medium and
this does not. #253 (replay window and host binding) is a different property.

Fix direction

now.saturating_sub(self.created).saturating_abs(), or reject an out-of-range created at parse time
so the value never reaches the arithmetic.

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:coregitlawb-core — identity, certs, encrypt, DID/UCANkind:bugDefect fix — wrong or unsafe behaviorsev:lowCosmetic, cleanup, or nice-to-havesubsystem: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