Skip to content

fix(domain): avoid overflow in bounded zlib decoding - #263

Merged
johnny4young merged 3 commits into
mainfrom
fix/zlib-output-limit-overflow
Oct 8, 2026
Merged

johnny4young merged 3 commits into
mainfrom
fix/zlib-output-limit-overflow

Conversation

@johnny4young

@johnny4young johnny4young commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Finding

The public bounded zlib decoder computes maxOutputBytes + 1 before clamping the chunk allocation. A valid Int.max ceiling therefore overflows for even a tiny compressed payload. Current share-link callers use a fixed smaller limit; this is defensive public-API correctness, not a demonstrated current-app crash.

Change

Clamp the nonnegative limit before adding the sentinel byte. The buffer remains at most 64 KiB, and decompression still rejects output beyond the caller's unchanged ceiling.

Regression coverage

Three native tests cover:

  • A tiny round trip with an Int.max output ceiling
  • Exact and insufficient limits across the 64 KiB chunk boundary
  • Empty/zero and negative limit error behavior

Deduplication

All 12 open PRs #251–262 were inventoried, including complete changed-file lists. None changes ShareLinkCoding.swift or adds these domain tests. This main-based correction is independent of the broad app review, CLI/file safety repairs, dependency reconciliation and UI profiling/optimization candidates. No performance benefit is claimed.

Validation and limits

Source head: a70420e
Tree: e519ea71127253d2149002c9b58a9d5ebfeec9ae
Main base: bb8fea0

  • git diff --check passes and the locally staged tree matches the uploaded tree exactly.
  • make format, make lint and make test were attempted. Portable lint prerequisites passed; strict Swift formatting/native compilation/tests are blocked locally by missing xcrun/Xcode/XcodeGen. Exact-head hosted qualification is recorded below.
  • The overflow is established by the original checked Int addition; native execution of the fixed implementation is qualified below; no native run of the overflowing original is claimed.
  • No website, dependency, workflow, threshold, release, license or existing PR branch changes. No manual workflow dispatch or CI expansion, merge, release or deployment.

Ready for human review after the exact-head qualification below.

Exact-head qualification

CI 37660531515 completed successfully for unchanged head a70420e: all five jobs passed.

  • All three new zlib limit tests passed natively on both supported macOS rows, including Int.max, 64 KiB boundaries, empty/zero and negative limits.
  • Strict formatting, Debug and universal Release builds, package/app tests, performance and golden gates passed.
  • Sequoia production coverage: 27,667/47,445 = 58.31%, above the unchanged 47.87% floor. Tahoe: 27,690/47,445 = 58.36%, above the unchanged 47.93% floor. Critical changed-line coverage: 1/1 = 100% on both rows.
  • Both ordinary UI lanes and their separate strict visual-evidence tours passed; each exported and validated all 53 required screenshots.
  • Linux static/site checks passed. No gate, threshold, baseline or expected skip was relaxed.

This qualifies this main-based correction only, not other open branches or the published app release. No merge, release or deployment.

@johnny4young
johnny4young marked this pull request as ready for review October 7, 2026 21:39
claude added 2 commits October 8, 2026 19:59
…imit

- Clamp the output limit to zero before sizing the chunk so a negative limit
  can never reach UnsafeMutablePointer.allocate with a negative capacity, while
  Int.max still avoids the sentinel-byte overflow.
- Build the empty raw DEFLATE stream by hand, cover -1, -2, and Int.min, and
  pin the 64 KiB boundary at 65535, 65536, and 65537 bytes.
@johnny4young
johnny4young merged commit 6287587 into main Oct 8, 2026
6 checks passed
@johnny4young
johnny4young deleted the fix/zlib-output-limit-overflow branch October 8, 2026 21:06
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