Skip to content

fix(types): prevent implicit 32-bit overflow panic and OOM DoS in checkUint64#453

Merged
bodgit merged 1 commit into
bodgit:mainfrom
hobeone:fix/oom-check
May 22, 2026
Merged

fix(types): prevent implicit 32-bit overflow panic and OOM DoS in checkUint64#453
bodgit merged 1 commit into
bodgit:mainfrom
hobeone:fix/oom-check

Conversation

@hobeone
Copy link
Copy Markdown
Contributor

@hobeone hobeone commented May 22, 2026

  • Enforce math.MaxInt bounds check in checkUint64 to prevent negative-size slice allocations and implicit integer conversions panicking on 32-bit architectures.
  • Enforce math.MaxUint32 bounds check in checkUint64 to prevent slice allocation length out of range panics on 64-bit architectures.
  • On 64-bit platforms, allocating a slice of e.g. 4,000,000,000 elements will immediately trigger an Out-Of-Memory (OOM) crash, leading to a Denial of Service.
  • Implement incremental slice allocation via min(count, 1024) and append in readBool, readSizes, readFolder, and readUnpackInfo to mitigate memory exhaustion DoS from maliciously large metadata counts. Any slice growth would only happen if the archive had more than 1024 files.

…ckUint64

- Enforce math.MaxInt bounds check in checkUint64 to prevent negative-size slice allocations and implicit integer conversions panicking on 32-bit architectures.
- Enforce math.MaxUint32 bounds check in checkUint64 to prevent slice allocation length out of range panics on 64-bit architectures.
- Implement incremental slice allocation via min(count, 1024) and append in readBool, readSizes, readFolder, and readUnpackInfo to mitigate memory exhaustion DoS from maliciously large metadata counts.
- Dynamic slice growth via append has negligible performance impact as 7z decompression CPU cycles completely dominate total execution time (99.9%+).
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 74.939% (+0.05%) from 74.889% — hobeone:fix/oom-check into bodgit:main

@bodgit
Copy link
Copy Markdown
Owner

bodgit commented May 22, 2026

Yeah, the bounds check was a, (what I thought was sufficient), crude guardrail to just stop make panicking when I first tried fuzzing. As you've seen, some of the code reads a 64-bit number which sizes a slice of things but can never be math.MaxUint64 big anyway due to Golang limits. I didn't consider 32-bit platforms at the time so your change makes sense.

The other changes look good to me, I think the default sizes are reasonable.

@bodgit bodgit merged commit c9e301e into bodgit:main May 22, 2026
5 of 6 checks passed
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.

3 participants