Skip to content

Bug: an out-of-range stored expiry aborts fx instead of reading as expired #445

Description

@beardthelion

Issue details

refreshDeadlineMs in src/core/auth/grok_session.zig:19 and src/core/auth/chatgpt_session.zig:19 subtracts the one minute safety margin with a plain -:

pub fn refreshDeadlineMs(expires_at_ms: i64) i64 {
    return @max(expires_at_ms - expiry_skew_ms, 0);
}

Nothing bounds expires_at_ms. It comes straight out of the stored auth file: parse reads it through requiredInteger (grok_session.zig:220 and :249, chatgpt_session.zig:211 and :239), which accepts any JSON integer and range-checks nothing. At expires_at_ms < i64.MIN + 60_000 the subtraction overflows and the process aborts. The @max reads as if it covers this, but it runs on a result the subtraction never produces.

The interesting part is the contrast with the rest of the same parser. A bad account_id, a string where the expiry belongs, a wrong version: each returns error.InvalidGrokAuthSession, and loadFromDir turns that into no session (grok_session.zig:131-137). This one field aborts instead.

oauth_session.zig:106 does the same arithmetic with a saturating -| and is unaffected.

Reproduced on 385f74e0 with ~/.fx/grok-auth.json written at mode 0600 under a temporary HOME:

{"version":1,"access_token":"a","refresh_token":"r","expires_at_ms":-9223372036854775808,"account_id":"acct"}

grok_session.load returns the session, and the first thing that asks whether it is stale panics:

thread panic: integer overflow
  grok_session.zig:19  in refreshDeadlineMs   return @max(expires_at_ms - expiry_skew_ms, 0);
  grok_session.zig:44  in expired             return refreshDeadlineMs(self.expires_at_ms) <= now_ms;

Both refresh modes reach it. loadAccess calls expired at grok_oauth.zig:508, and takeAccess calls refreshDeadlineMs at :522, so .stored panics too without ever consulting the expiry for a decision. ChatGPT is the same at chatgpt_oauth.zig:407 and :421.

Normal values are unaffected: 100000 gives 40000, 10000 and 0 and -1 all clamp to 0, and i64.MAX does not overflow.

For scope: this is reachable from a corrupt or hand-edited auth file, not from a hostile server. The write path is already guarded, since expires_in goes through requiredPositiveInteger and then checked mul and add, so a token response cannot produce a value in the overflow range. What it costs is that one damaged file turns every fx invocation that touches Grok or ChatGPT auth into an abort, where every other kind of damage to the same file degrades to a normal re-login.

The existing tests cover 100000 and 10000 only (grok_session.zig:289-290, chatgpt_session.zig:267-268), which is why the boundary was never exercised.

Sensitive information

  • I reviewed the information above and removed secrets and other sensitive data.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions