Skip to content

fix(grok): reject unsafe protobuf response lengths - #514

Open
luvs01 wants to merge 27 commits into
devfrom
codex/fix-denial-of-service-vulnerability-in-parser
Open

luvs01 wants to merge 27 commits into
devfrom
codex/fix-denial-of-service-vulnerability-in-parser

Conversation

@luvs01

@luvs01 luvs01 commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Motivation

  • The hand-written protobuf varint decoder used bitwise shifts that coerced results to signed 32-bit integers, allowing a crafted Grok response to produce a negative length and drive a synchronous CPU denial-of-service.
  • The decoder and top-level parser trusted provider-controlled lengths and advanced offsets without validating non-negativity, safe-integer representation, or bounds inside the enclosing message.

Description

  • Replace signed 32-bit bitwise accumulation in decodeVarint with safe-number arithmetic and validate the offset argument by throwing on invalid inputs.
  • Reject truncated or unsafe varints by throwing on overflow/unsafe integers and on incomplete encodings instead of returning a corrupted value.
  • Add a decodeLength helper that validates a length-delimited field (length is non-negative, a safe integer, and does not exceed the remaining bytes) and use it in decodeTimestamp, decodeConsumerResetToken, and decodeGetRemainingResetsResponse so parser offsets advance only within bounds.
  • Add a regression test that asserts the reported malicious 2^31 length payload is rejected and that truncated varints throw, and document the src/grok/ parsing contract in structure/runtime.md.

Testing

  • Ran bun test tests/providers/xai/grok-reset-coupons.test.ts, which passed (7 tests, focused suite).
  • Ran bun run structure:check, bun run typecheck, and bun run privacy:scan, all of which passed.
  • Attempted bun run test (full suite); the focused security regression passed, but the full parallel run encountered unrelated worker panics and admission-test failures in other domains, so the repository-wide run did not complete cleanly.
  • Verified git diff --check and the local changes compile and the focused regression prevents the previously reported exploit input from causing synchronous infinite parsing.

Codex Task

lidge-jun and others added 27 commits September 6, 2026 01:08
…43-01a07240

[WRONG BRANCH] chore(release): promote verified candidate to 2.43.0
…in-07c0

chore(release): promote validated 2.44.0 to main
Promote frozen dev source cf9f662; no new runtime changes. Candidate CI34061274315 and service34061276621 are the validation references. Publication waits for successful validation and the final main push CI at the exact release SHA.
Promote frozen dev source cf9f662 as 2.45.0.

The repository owner explicitly authorized this main/preview release promotion and admin PR-only merge. This is a release-specific owner decision, not an independent approval or the dev-only maintainer exception.

Frozen candidate full CI34061274315 passed all25jobs after one unchanged-source rerun of Windows5; the initial holder busy assertion remains recorded without a root-cause resolution claim. Service lifecycle34061276621 passed Linux/macOS/Windows. Dev version pre-move3812 is merged. Publication still requires this actual main merge SHA's own successful push CI and Service lifecycle. No local suites were run.
[WRONG BRANCH] chore(release): promote verified 2.46.0 to main
[WRONG BRANCH] release: promote 2.47.0 to main
[WRONG BRANCH] release: apply final roster correction to main
[WRONG BRANCH] release: promote 2.48.0 to main
…in-01a08498

release: promote verified 2.49.0 product tree to main
Promotes the audited freeze SHA 12c248f unchanged. The resulting tree is byte-identical to that commit's tree, d8f5a71, so what ships is exactly what the six-lane regression audit read.
…in-01a08a81

[WRONG BRANCH] release: promote verified 2.50.0 product tree to main
[WRONG BRANCH] release: promote verified 2.51.0 product tree to main
Product tree is dev at 7bfb2ad. Only package.json differs, carrying the
stable channel version 2.52.0.

This promotion follows a CI repair and a regression sweep: dev had drifted 79
commits past its last finished run and the one run allowed to complete had
failed. lidge-jun#4390 fixed an integration test that never denied anything and had been
red on dev since lidge-jun#4342; lidge-jun#4396/lidge-jun#4397/lidge-jun#4398 closed four gaps in the quota
avoidance contract lidge-jun#4368 introduced; lidge-jun#4403 made a port fixture deterministic.
release: promote verified 2.52.0 product tree to main
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8b4c5b26-f77c-46e3-b72a-1a92a9f495d6


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T00:24:10.610069Z c2e5e77 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@luvs01
luvs01 changed the base branch from main to Dev September 14, 2026 00:22
@github-actions github-actions Bot added the bug Something isn't working label Sep 14, 2026
@github-actions

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

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

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants