Fix infinite loop / unbounded allocation in readArray on truncated PDFs - #79
Open
spencerkimball wants to merge 1 commit into
Open
Fix infinite loop / unbounded allocation in readArray on truncated PDFs#79spencerkimball wants to merge 1 commit into
spencerkimball wants to merge 1 commit into
Conversation
readToken returns io.EOF as a token value once the input is exhausted,
but readArray only broke out of its loop on nil or keyword("]"). On a
PDF whose content stream is truncated inside an unterminated array, the
loop unread the io.EOF token, read it back as an object, and appended
it to the array forever, allocating memory without bound (~7GB of heap
in 5 seconds observed on a real-world malformed PDF) and hanging every
text-extraction entry point (GetPlainText, Page.Content,
Page.GetTextByRow).
readDict already guards against io.EOF; give readArray the same guard.
Add a regression test that synthesizes a minimal single-page PDF whose
content stream ends inside an unterminated array and verifies that
GetPlainText returns instead of spinning. The test times out against
the previous code and passes with the fix.
|
I faced the same issue |
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
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.
The bug
buffer.readArrayinlex.goloops reading tokens until it seesnilorkeyword("]"):When the input is exhausted,
readTokenreturnsio.EOFas a token value (if b.eof { return io.EOF }), which matches neither break condition. The loop then unreads theio.EOFtoken,readObjectreads it back and returns it as an object, and it gets appended to the array — forever. Each iteration allocates, so memory grows without bound until the process is OOM-killed.readDictin the same file already has an explicitio.EOFguard;readArraywas missing the equivalent.Impact
Any PDF whose content stream is truncated inside an unterminated array (
[with no closing]) hangs every text-extraction entry point:Reader.GetPlainText,Page.GetPlainText,Page.Content,Page.GetTextByRow,Reader.GetStyledTexts— they all reachreadArrayviaInterpret. This is not just a theoretical fuzz case: we hit it on a real-world malformed PDF while batch-extracting text from email attachments, and measured roughly 7 GB of heap growth in about 5 seconds before the process was killed.The fix
Break out of the loop on
io.EOFas well, mirroringreadDict's handling — a one-line condition change. I audited the otherreadToken/readObjectloops for the same shape:readDict(lex.go) andInterpret(ps.go) already break onio.EOF, and the loops in read.go either error out on a failed type assertion or are iteration-bounded, so no other changes were needed.Regression test
lex_test.gosynthesizes a minimal single-page PDF entirely in code (no external files) whose content stream ends inside an unterminated array, then runsGetPlainTextin a goroutine with a 5-second watchdog. The test was verified to fail against unpatched master:and passes with the fix (returns immediately).
go test ./...passes.