Cap ObjStm header prealloc to bound /N memory amplification#13
Merged
Conversation
readCompressedObject sized its (objNum, offset) slice from the ObjStm's /N entry, which is attacker-controlled and bounded only by the decoded stream length. A few-KB PDF whose ObjStm inflates to 16 MiB of zeros and declares /N = 16M forced a ~256 MiB allocation before a single header entry was parsed. Cap the prealloc; append grows it to the real count. Adds regression tests at the Open()/Resolve() surface: an allocation bound for the hostile case and a >cap ObjStm that still resolves correctly, plus xref-recovery coverage (catalog-scan recovery and a /Prev incremental-update merge).
fank
force-pushed
the
claude/objstm-prealloc-dos
branch
from
26d039d to
5ef8326
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
readCompressedObjectpreallocated its(objNum, offset)slice with the ObjStm's/Ncount:/Nis attacker-controlled and bounded only by the decoded stream length. With the default 16 MiB stream cap, a few-KB PDF — an ObjStm that FlateDecode-inflates to 16 MiB of zeros and declares/N = 16777216— forces a ~256 MiB allocation (16M × sizeof(pair)) before a single header entry is parsed. The header parse then fails, but the memory is already committed. Memory-amplification DoS, directly in the untrusted-input threat model.Fix
Cap the prealloc to a small constant;
appendgrowspairsto the real entry count (which the header-parse loop already bounds by bytes actually present).Tests (at the
Open()/Resolve()surface)TestObjStmHugeNDoesNotAmplifyAllocation— red before the fix (304 MiB measured), green after (bounded under a 128 MiB ceiling viaruntime.MemStatsTotalAllocdelta).TestObjStmManyObjectsResolvePastPrealloc— a legitimate ObjStm with 5000 entries (> the cap) still resolves an object past the cap to its exact value, provingappendgrowth doesn't truncate.Also adds xref-recovery coverage found while auditing this path:
TestRecoverXrefViaCatalogScan— no startxref / notrailerkeyword; recovery scans rebuilt objects for/Type /Catalog.TestPrevChainNewestSectionWins—/Previncremental-update chain; newest section wins and the older trailer's keys survive the merge.Coverage (main package): 73.7% → 76.1%;
recoverXref75% → 91%.