Created attachment 131001 [details] testcase on poppler 0.54.0 The GfxImageColorMap::getGray function in GfxState.cc:6064 allows attackers to cause a denial of service (stack buffer overflow) via a crafted file. #pdfimages $FILE out ==88072==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffef185eb1 at pc 0x0000004fd590 bp 0x7fffef185cd0 sp 0x7fffef185cc8 READ of size 1 at 0x7fffef185eb1 thread T0 #0 0x4fd58f in GfxImageColorMap::getGray(unsigned char*, int*) /home/haojun/Downloads/testopensourcecode/poppler/poppler/GfxState.cc:6064 #1 0x408407 in ImageOutputDev::writeImageFile(ImgWriter*, ImageOutputDev::ImageFormat, char const*, Stream*, int, int, GfxImageColorMap*) /home/haojun/Downloads/testopensourcecode/poppler/utils/ImageOutputDev.cc:386 #2 0x40a557 in ImageOutputDev::writeImage(GfxState*, Object*, Stream*, int, int, GfxImageColorMap*, bool) /home/haojun/Downloads/testopensourcecode/poppler/utils/ImageOutputDev.cc:647 #3 0x40a9d1 in ImageOutputDev::drawSoftMaskedImage(GfxState*, Object*, Stream*, int, int, GfxImageColorMap*, bool, Stream*, int, int, GfxImageColorMap*, bool) /home/haojun/Downloads/testopensourcecode/poppler/utils/ImageOutputDev.cc:703 #4 0x4a7630 in Gfx::doImage(Object*, Stream*, bool) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Gfx.cc:4702 #5 0x4a445f in Gfx::opXObject(Object*, int) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Gfx.cc:4208 #6 0x47efd0 in Gfx::execOp(Object*, Object*, int) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Gfx.cc:904 #7 0x47e091 in Gfx::go(bool) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Gfx.cc:763 #8 0x47dbec in Gfx::display(Object*, bool) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Gfx.cc:729 #9 0x52c8f6 in Page::displaySlice(OutputDev*, double, double, int, bool, bool, int, int, int, int, bool, bool (*)(void*), void*, bool (*)(Annot*, void*), void*, bool) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Page.cc:601 #10 0x52be69 in Page::display(OutputDev*, double, double, int, bool, bool, bool, bool (*)(void*), void*, bool (*)(Annot*, void*), void*, bool) /home/haojun/Downloads/testopensourcecode/poppler/poppler/Page.cc:523 #11 0x533614 in PDFDoc::displayPage(OutputDev*, int, double, double, int, bool, bool, bool, bool (*)(void*), void*, bool (*)(Annot*, void*), void*, bool) /home/haojun/Downloads/testopensourcecode/poppler/poppler/PDFDoc.cc:494 #12 0x5336b9 in PDFDoc::displayPages(OutputDev*, int, int, double, double, int, bool, bool, bool, bool (*)(void*), void*, bool (*)(Annot*, void*), void*) /home/haojun/Downloads/testopensourcecode/poppler/poppler/PDFDoc.cc:510 #13 0x406119 in main /home/haojun/Downloads/testopensourcecode/poppler/utils/pdfimages.cc:218 #14 0x7fecc4ca0b34 in __libc_start_main (/lib64/libc.so.6+0x21b34) #15 0x4059a8 (/home/haojun/Downloads/testopensourcecode/poppler-build/bin/pdfimages+0x4059a8) Address 0x7fffef185eb1 is located in stack of thread T0 at offset 33 in frame #0 0x407fa3 in ImageOutputDev::writeImageFile(ImgWriter*, ImageOutputDev::ImageFormat, char const*, Stream*, int, int, GfxImageColorMap*) /home/haojun/Downloads/testopensourcecode/poppler/utils/ImageOutputDev.cc:338 This frame has 5 object(s): [32, 33) 'zero' <== Memory access at offset 33 overflows this variable [96, 100) 'gray' [160, 168) 'row' [224, 236) 'rgb' [288, 304) 'cmyk' HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow /home/haojun/Downloads/testopensourcecode/poppler/poppler/GfxState.cc:6064 in GfxImageColorMap::getGray(unsigned char*, int*) Shadow bytes around the buggy address: 0x10007de28b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007de28b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007de28ba0: 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 00 00 0x10007de28bb0: 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 0x10007de28bc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x10007de28bd0: 00 00 f1 f1 f1 f1[01]f4 f4 f4 f2 f2 f2 f2 04 f4 0x10007de28be0: f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 04 0x10007de28bf0: f4 f4 f2 f2 f2 f2 00 00 f4 f4 f3 f3 f3 f3 00 00 0x10007de28c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007de28c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007de28c20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==88072==ABORTING The $FILE poc in the attachment. Credit:The bug was discovered by Haojun Hou in ADLab of Venustech.
Adrian, this patch fixes it http://paste.ubuntu.com/24455566/ But i'm not sure this does exactly what you had in mind with the code for invert_bits detecting introduced at e53aec2c61ba42cf0635dc05f8e27e3503c1eaac Can you have a look?
The code is checking what color the value zero maps to. The existing call to colorMap->getGray(&zero, &gray); should be fine as GfxImageColorMap::getGray() handles colorSpace2 internally. Instead of adding a getNumPixelComps2() function it would better to check if colorSpace2 is not null and handle it inside getNumPixelComps() like is done with some of the other functions in GfxImageColorMap. The value returned by getNumPixelComps() should match the number of components returned by getGray() and getRGB(). getGray() and getRGB() check for and use colorSpace2 so getNumPixelComps() should return colorSpace2 num components when colorSpace2 is not null.
I don't think it's a good idea to change getNumPixelComps, there's too many uses of it, are we sure all of them really want a different value when there colorSpace2 is not null?
Hi,have you fixed this issue in the latest version?
Created attachment 132633 [details] [review] how about just this simple approach to make zero equal to the largest possibly required size
Pushed
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.