Repository navigation
pclmtoraster: reject a page with an unsupported /Rotate value - #277
Open
ManasBagul23 wants to merge 1 commit into
Open
ManasBagul23 wants to merge 1 commit into
ManasBagul23 wants to merge 1 commit into
Conversation
rotate_bitmap() only implements rotations of 90, 180 and 270 degrees. For
any other nonzero value it logs an error and returns its *src* argument
unchanged -- not the *dst* buffer the caller allocated for the rotated
result.
out_page() cannot tell the two outcomes apart: it always treats the
return value as the new bitmap, frees the original data->bitmap, and
assigns the (for an unsupported rotation, identical) returned pointer to
data->bitmap. That frees the buffer the caller is about to keep using
(use-after-free, read by every row of the subsequent raster-conversion
loop), and the function's own end-of-page cleanup frees data->bitmap a
second time (double free). The destination buffer allocated right before
the call is also leaked, since the local holding it is overwritten with
the return value.
/Rotate is read straight from the PDF page dictionary with no range
check, so a page declaring any value outside {0, 90, 180, 270} (e.g. 45)
reaches this.
Reject the page as soon as /Rotate is read, before the allocate/rotate/
free sequence that depends on it ever runs -- matching the function's
existing `return (1)` convention, used a few lines below for an invalid
mediaBox.
Verified by tracing rotate_bitmap()'s only non-aliasing-safe return paths
(90/180/270, which fill and return the actual `dst`) against the one
aliasing path this guard removes (the `else` branch, which returns `src`)
and confirming out_page()'s use of the return value only assumes the
former. No C build toolchain available locally to compile-check.
Fixes OpenPrinting#186
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.
Fixes #186
rotate_bitmap()only implements rotations of 90, 180 and 270 degrees. For any other nonzero value it logs an error and returns itssrcargument unchanged — not thedstbuffer the caller allocated for the rotated result.out_page()cannot tell the two outcomes apart: it always treats the return value as the new bitmap, frees the originaldata->bitmap, and assigns the (for an unsupported rotation, identical) returned pointer todata->bitmap. That frees the buffer the caller is about to keep using (use-after-free, read by every row of the subsequent raster-conversion loop), and the function's own end-of-page cleanup freesdata->bitmapa second time (double free). The destination buffer allocated right before the call is also leaked, since the local holding it is overwritten with the return value./Rotateis read straight from the PDF page dictionary with no range check, so a page declaring any value outside{0, 90, 180, 270}(e.g. 45) reaches this.Reject the page as soon as
/Rotateis read, before the allocate/rotate/free sequence that depends on it ever runs — matching the function's existingreturn (1)convention, used a few lines below for an invalid mediaBox.Verified by tracing
rotate_bitmap()'s only non-aliasing-safe return paths (90/180/270, which fill and return the actualdst) against the one aliasing path this guard removes (theelsebranch, which returnssrc) and confirmingout_page()'s use of the return value only assumes the former.