Stop shipping two denial-of-service defects to our callers - #17
Merged
Conversation
This module required reader v0.4.0 and pdffont v0.2.0. Both had already been
fixed and released; nothing had brought the fixes here, so a caller who asked
only for this package got the defects — minimum version selection gives them
the version this go.mod names, and a package's own tests never see its callers'
module graph. Merging is not shipping.
reader v0.4.1 a 219-byte file with no trailer and no startxref, whose last
object is numbered 2 147 483 647, took 21.2 s to open and
allocated not one byte — so no memory limit caught it.
Now under a millisecond.
pdffont v0.3.0 a /ToUnicode map's size had nothing to do with its input:
10 655 bytes produced 13 107 200 entries, 1 054 MB and 10 s,
per font per page. Now 52 ms and 31 MB. Also the glyph names
of eight more languages, which had been read as nothing at
all — "Příliš" came back "Píliš" and still looked like a word.
deps_test.go guards the first of those with the file itself rather than with a
version string, so a later go.mod edit cannot quietly reintroduce it. It is
built in the test, not committed, so nobody else's PDF enters the repository.
Against reader v0.4.0 it fails after five seconds.
WHAT IT CHANGES ON THE CORPUS
Two binaries, identical but for pdffont, verified with `go version -m` because
this measurement has silently compared a version against itself twice on this
project. 8 935 real files — 2 268 forms and 6 667 arXiv papers — first page
each, 20-second budget:
identical 8 929
differing 6
Of the six, three are pages sitting at the budget: with a MaxDuration set, a
page near the limit stops at a different operation each run, so its output is
timing-dependent and nothing can be concluded from it. One is safedocs'
Inline_Image_Abbreviations fixture, which disagrees with itself between runs of
the same binary for an unrelated reason (go-pdfkit/reader#14).
That leaves two genuine changes, both in the vendor half of the forms corpus:
gh-pdfbox/source.pdf +142 inked pixels
gh-pdfbox/sample_fonts_solidconvertor -58 inked pixels, all inside one
152x212 region, 1 003 pixels changed
The second is the interesting one and it is not fully explained: the file has
no /Differences and no uniXXXX names anywhere, decompressed streams included,
so the difference cannot come from a document-supplied glyph name. It arrives
through the base-encoding table, where the added letters include the ones
WinAnsiEncoding already names — scaron, zcaron, oe. Fewer inked pixels is
consistent with a real narrow glyph replacing a wider wrong one, but that is
inference and not measurement, so it is written down as unexplained rather than
as an improvement.
No file got worse in a way that can be shown. Wall clock is not compared: four
other jobs had the machine while this ran, and a number taken under that load
would be fiction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
This module required reader v0.4.0 and pdffont v0.2.0. Both had already been
fixed and released; nothing had brought the fixes here, so a caller who asked
only for this package got the defects — minimum version selection gives them
the version this go.mod names, and a package's own tests never see its callers'
module graph. Merging is not shipping.
reader v0.4.1 a 219-byte file with no trailer and no startxref, whose last
object is numbered 2 147 483 647, took 21.2 s to open and
allocated not one byte — so no memory limit caught it.
Now under a millisecond.
pdffont v0.3.0 a /ToUnicode map's size had nothing to do with its input:
10 655 bytes produced 13 107 200 entries, 1 054 MB and 10 s,
per font per page. Now 52 ms and 31 MB. Also the glyph names
of eight more languages, which had been read as nothing at
all — "Příliš" came back "Píliš" and still looked like a word.
deps_test.go guards the first of those with the file itself rather than with a
version string, so a later go.mod edit cannot quietly reintroduce it. It is
built in the test, not committed, so nobody else's PDF enters the repository.
Against reader v0.4.0 it fails after five seconds.
WHAT IT CHANGES ON THE CORPUS
Two binaries, identical but for pdffont, verified with
go version -mbecausethis measurement has silently compared a version against itself twice on this
project. 8 935 real files — 2 268 forms and 6 667 arXiv papers — first page
each, 20-second budget:
identical 8 929
differing 6
Of the six, three are pages sitting at the budget: with a MaxDuration set, a
page near the limit stops at a different operation each run, so its output is
timing-dependent and nothing can be concluded from it. One is safedocs'
Inline_Image_Abbreviations fixture, which disagrees with itself between runs of
the same binary for an unrelated reason (go-pdfkit/reader#14).
That leaves two genuine changes, both in the vendor half of the forms corpus:
gh-pdfbox/source.pdf +142 inked pixels
gh-pdfbox/sample_fonts_solidconvertor -58 inked pixels, all inside one
152x212 region, 1 003 pixels changed
The second is the interesting one and it is not fully explained: the file has
no /Differences and no uniXXXX names anywhere, decompressed streams included,
so the difference cannot come from a document-supplied glyph name. It arrives
through the base-encoding table, where the added letters include the ones
WinAnsiEncoding already names — scaron, zcaron, oe. Fewer inked pixels is
consistent with a real narrow glyph replacing a wider wrong one, but that is
inference and not measurement, so it is written down as unexplained rather than
as an improvement.
No file got worse in a way that can be shown. Wall clock is not compared: four
other jobs had the machine while this ran, and a number taken under that load
would be fiction.