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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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