Add index safety checks in apply_png_predictor() - #1270
Conversation
|
Note that #1271 resolves the source of the error here, and is probably the more correct fix. However, the patch I have here can't hurt as well, for safety in unforeseen cases which may throw IndexError here. I would vote for merging both patches. |
eeshsaxena
left a comment
There was a problem hiding this comment.
Thanks for tackling the IndexError. One concern: for filter types 3 and 4 the new code gates the output raw.append(...) on 0 <= j < len(line_above), which silently truncates the decoded scanline whenever len(line_above) < len(line_encoded).
That is not just a malformed-input case, it happens on the first scanline of any image where nbytes > columns, i.e. colors > 1 (RGB, CMYK, etc). line_above is initialised to bytearray(columns), but line_encoded has length nbytes = colors * columns * bitspercomponent // 8, so on the first row len(line_above) = columns < nbytes.
Concrete repro on this branch (RGB, Colors=3, Columns=2, filter type 3 Average):
from pdfminer.utils import apply_png_predictor
data = bytes([3]) + bytes([10, 20, 30, 40, 50, 60]) # filter=3 + 6 encoded bytes
out = apply_png_predictor(1, colors=3, columns=2, bitspercomponent=8, data=data)
# expected len 6, actual len 2 -> [10, 20]So the first RGB row decodes to 2 bytes instead of 6, and every subsequent row is then misaligned. On main the same input raises IndexError (line_above[j] with j up to nbytes-1), so this trades a crash for silent image corruption.
The underlying issue is the initialisation: line_above = bytearray(columns) should be bytearray(nbytes) (the prior scanline is all zeros and is nbytes long, not columns). With that, line_above[j] is always in range, no bounds gymnastics are needed, and the output keeps the correct length. If you prefer to keep the guards, the fix is to treat a missing prior byte as 0 and still append, e.g. prior_x = line_above[j] if j < len(line_above) else 0, rather than skipping the append. Note filter type 2 (Up) has the same latent truncation via zip(..., strict=False).
Closes bug #1269.