heap out of bounds read (large) in Pl_Buffer::write

[hannob]

The attached file causes an out of bounds heap read, detectable with asan, found with libfuzzer.
qpdf-heapoob-Pl_Buffer_write.zip

ASAN error:

==3624==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000b31 at pc 0x0000004bcf25 bp 0x7ffe120cea30 sp 0x7ffe120ce1e0
READ of size 4294967295 at 0x602000000b31 thread T0
    #0 0x4bcf24 in __asan_memcpy (/r/qpdf/qpdf+0x4bcf24)
    #1 0x8145c4 in Pl_Buffer::write(unsigned char*, unsigned long) /f/qpdf/qpdf/libqpdf/Pl_Buffer.cc:21:5
    #2 0x821a14 in Pl_PNGFilter::decodeRow() /f/qpdf/qpdf/libqpdf/Pl_PNGFilter.cc:109:16
    #3 0x820ddf in Pl_PNGFilter::processRow() /f/qpdf/qpdf/libqpdf/Pl_PNGFilter.cc:69:2
    #4 0x820ddf in Pl_PNGFilter::write(unsigned char*, unsigned long) /f/qpdf/qpdf/libqpdf/Pl_PNGFilter.cc:43
    #5 0x819677 in Pl_Flate::handleData(unsigned char*, int, int) /f/qpdf/qpdf/libqpdf/Pl_Flate.cc:155:24
    #6 0x818e6c in Pl_Flate::write(unsigned char*, unsigned long) /f/qpdf/qpdf/libqpdf/Pl_Flate.cc:74:9
    #7 0x5e12b0 in QPDF::pipeStreamData(int, int, long long, unsigned long, QPDFObjectHandle, Pipeline*, bool) /f/qpdf/qpdf/libqpdf/QPDF.cc:2411:16
    #8 0x70625e in QPDF::Pipe::pipeStreamData(QPDF*, int, int, long long, unsigned long, QPDFObjectHandle, Pipeline*, bool) /f/qpdf/qpdf/include/qpdf/QPDF.hh:559:19
    #9 0x70625e in QPDF_Stream::pipeStreamData(Pipeline*, unsigned long, qpdf_stream_decode_level_e, bool) /f/qpdf/qpdf/libqpdf/QPDF_Stream.cc:533
    #10 0x702b16 in QPDF_Stream::getStreamData(qpdf_stream_decode_level_e) /f/qpdf/qpdf/libqpdf/QPDF_Stream.cc:90:11
    #11 0x606ed6 in QPDFObjectHandle::getStreamData(qpdf_stream_decode_level_e) /f/qpdf/qpdf/libqpdf/QPDFObjectHandle.cc:488:58
    #12 0x5a5309 in QPDF::processXRefStream(long long, QPDFObjectHandle&) /f/qpdf/qpdf/libqpdf/QPDF.cc:1009:41
    #13 0x59488c in QPDF::read_xrefStream(long long) /f/qpdf/qpdf/libqpdf/QPDF.cc:893:20
    #14 0x56f538 in QPDF::read_xref(long long) /f/qpdf/qpdf/libqpdf/QPDF.cc:523:20
    #15 0x568e3f in QPDF::parse(char const*) /f/qpdf/qpdf/libqpdf/QPDF.cc:328:2
    #16 0x565e1a in QPDF::processFile(char const*, char const*) /f/qpdf/qpdf/libqpdf/QPDF.cc:141:5
    #17 0x51853d in main /f/qpdf/qpdf/qpdf/qpdf.cc:2300:17
    #18 0x7f8d7e6e94f0 in __libc_start_main (/lib64/libc.so.6+0x204f0)
    #19 0x41d459 in _start (/r/qpdf/qpdf+0x41d459)

0x602000000b31 is located 0 bytes to the right of 1-byte region [0x602000000b30,0x602000000b31)
allocated by thread T0 here:
    #0 0x50c320 in operator new[](unsigned long) (/r/qpdf/qpdf+0x50c320)
    #1 0x8207e3 in Pl_PNGFilter::Pl_PNGFilter(char const*, Pipeline*, Pl_PNGFilter::action_e, unsigned int, unsigned int) /f/qpdf/qpdf/libqpdf/Pl_PNGFilter.cc:17:18

[jberkenbilt]

I've reproduced this in the test suite on my branch. I'll fix it next time I have a chance, hopefully tomorrow sometime.

[jberkenbilt]

I've reproduced this in the test suite on my branch. I'll fix it next time I have a chance, hopefully tomorrow sometime.

[jberkenbilt]

This was an integer overflow. I have updated the code to, I believe, detect integer overflow/underflow in all cases now. This will be on master as soon as I test on Windows. It works in Linux with clang and gcc and the whole test suite, with the addition of this file and all the other ones you've provided, runs clean through address sanitizer on my work branch.

[jberkenbilt]

This was an integer overflow. I have updated the code to, I believe, detect integer overflow/underflow in all cases now. This will be on master as soon as I test on Windows. It works in Linux with clang and gcc and the whole test suite, with the addition of this file and all the other ones you've provided, runs clean through address sanitizer on my work branch.

[ghost]

This has been assigned CVE-2017-18185

[ghost]

This has been assigned CVE-2017-18185

[kirotawa]

Hi,

What is the commit that fix this issue? I saw that commit in closed is just adding tests. Is that the fix itself?

[kirotawa]

Hi,

What is the commit that fix this issue? I saw that commit in closed is just adding tests. Is that the fix itself?

[jberkenbilt]

@kirotawa I would have to do some investigation to figure out exactly which one it is. I fixed this series of issues by addressing some fundamental items in the code, such as detecting integer overflows, and some of the fixes to the code fixed multiple issues. Most likely 6d46346 or 1868a10 is responsible for the actual fix.

[jberkenbilt]

@kirotawa I would have to do some investigation to figure out exactly which one it is. I fixed this series of issues by addressing some fundamental items in the code, such as detecting integer overflows, and some of the fixes to the code fixed multiple issues. Most likely 6d46346 or 1868a10 is responsible for the actual fix.