Skip to content

Bug: failed session write during provider token refresh double-frees the refresh token and account id #436

Description

@beardthelion

Issue details

refreshSession in src/core/auth/grok_oauth.zig and src/core/auth/chatgpt_oauth.zig arms three errdefers over the same two allocations before it persists the refreshed session:

  • grok_oauth.zig:542 / chatgpt_oauth.zig:442: errdefer alloc.free(account_id)
  • grok_oauth.zig:548 / chatgpt_oauth.zig:448: errdefer secret.zeroAndFree(alloc, refresh_token)
  • grok_oauth.zig:562 / chatgpt_oauth.zig:462: errdefer replacement.deinit(alloc)

replacement is built from those same two slices, and Session.deinit releases all three of its fields. So when mutation.save on the next line fails, unwinding runs replacement.deinit first and frees everything, then the two older errdefers fire on memory that is already gone. The refresh token also gets secure-zeroed into the freed block on the way, since zeroAndFree writes before it releases.

The success path is correct, so this is confined to the save failure edge. That edge is an ordinary disk condition rather than a hypothetical: an unwritable ~/.fx, a full disk, or an I/O error is enough, and a background refresh can hit it with no user action.

Reproduced against std.testing.allocator by pointing the mutation at a directory with mode 0500 so the save fails in durableReplaceVerified. Both providers report two double frees:

Double free detected. Allocation: grok_oauth.zig:538   <- refresh_token
 First free:  grok_oauth.zig:562  errdefer replacement.deinit(alloc)
 Second free: grok_oauth.zig:548  errdefer secret.zeroAndFree(alloc, refresh_token)

Double free detected. Allocation: grok_oauth.zig:541   <- account_id
 First free:  grok_oauth.zig:562  errdefer replacement.deinit(alloc)
 Second free: grok_oauth.zig:542  errdefer alloc.free(account_id)

chatgpt_oauth.zig is identical at 438/441, 462, 448, 442. The access token is fine: token.access_token = &.{} leaves replacement its only owner.

Nothing exercised refreshSession, which is why this was not caught. The in-file tests cover requestRefreshToken response parsing only, and tests/e2e/auth-refresh.test.ts runs the Vercel issuer on paths where the save succeeds.

Not affected: credentials.refreshFxSession (the Vercel path) updates the session fields in place with no replacement struct, and the other errdefer sites in both files keep one owner per slice.

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