Skip to content

fix(core): avoid grade stats regex backtracking - #3706

Merged
jrusso1020 merged 1 commit into
mainfrom
fix/security-grade-stats-regex
Sep 5, 2026
Merged

fix(core): avoid grade stats regex backtracking#3706
jrusso1020 merged 1 commit into
mainfrom
fix/security-grade-stats-regex

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

Fixes CodeQL alert #796: malformed FFmpeg signal-stat headers containing long digit runs cause quadratic regex backtracking.

Require one initial frame digit instead of a repeated digit group overlapping .*. The existing wildcard already consumes all remaining digits, preserving accepted headers and the last valid pts_time capture. Apply the same change to the standalone media-use skill copy. No process invocation or grading logic changes.

Validation: the original regex exceeded a two-second subprocess timeout on 100,000 digits. All 15 product/parity tests and 10 standalone skill tests (including real FFmpeg cases) pass. New coverage checks long malformed and valid headers, frame boundaries, signed/fractional timestamps, duplicate timestamps, numeric prefixes and invalid headers. Parser/core builds, core/runtime typechecks, lint, format and commit hooks pass.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 8e349e113a286f828d0c3b6195c9c171f387b8bf; no blockers.

The patch follows CodeQL js/polynomial-redos's recommendation by removing the ambiguity itself. ^frame:\d.*pts_time: preserves the old language: both forms require one initial frame digit, and the existing wildcard accepts the remaining digits and separator text (packages/core/src/mediaGradeAnalyzer.ts:160-162; standalone copy skills/media-use/scripts/lib/grade-analyzer.mjs:79-81). The timestamp capture is unchanged.

I compared old/new captures across 500,000 deterministic random headers plus 200,000 structured headers: zero mismatches. Duplicate behavior is preserved: two valid pts_time fields select the last; an invalid trailing field falls back to the earlier valid field. Signed/fractional values and the existing numeric-prefix behavior are unchanged. The malformed 100k-digit case completes in ~0.17 ms with the new expression; one million digits takes ~1.9 ms.

The parity test exercises the product and standalone implementations together across valid/malformed boundaries, duplicate timestamps, Unicode line separation, long invalid digits, and a long valid header (packages/core/src/mediaGradeAnalyzer.vendoredParity.test.ts:104-124). I independently recomputed the full media-use bundle fingerprint: 5b35f62a2c59f220 over 153 files, exactly matching skills-manifest.json.

CI is still running at this head; initial lint/skill-manifest checks are green and JavaScript CodeQL plus longer build/render/regression jobs remain landing gates. Left unmerged as requested.

Verdict: APPROVE
Reasoning: The ambiguous repetition is removed without changing captured timestamps, both shipped copies are parity-tested, and the generated skill manifest matches the changed bundle exactly.

— Magi

@jrusso1020
jrusso1020 merged commit eec0434 into main Sep 5, 2026
59 checks passed
@jrusso1020
jrusso1020 deleted the fix/security-grade-stats-regex branch September 5, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants