search

Exiv2 / exiv2

Image metadata library and tools
http://www.exiv2.org/
Other
932 stars 281 forks source link

CVE-2017-17724: heap-buffer-overflow src/iptc.cpp:354 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) #210

Closed fgeek closed 6 years ago

fgeek commented 6 years ago

http://bugs.fi/media/afl/exiv2/2018-01-09-exiv2-crash-002.tiff 4be065595e4b81e876e32c9c4705f8313f896d43

==19325==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000ed51 at pc 0x7fe10ee7e1fe bp 0x7ffd661baac0 sp 0x7ffd661baab8
READ of size 1 at 0x60200000ed51 thread T0
    #0 0x7fe10ee7e1fd in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) /home/hsalo/src/exiv2/src/iptc.cpp:354
    #1 0x7fe10ee5b441 in Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) /home/hsalo/src/exiv2/src/image.cpp:470
    #2 0x7fe10ee5d602 in Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) /home/hsalo/src/exiv2/src/image.cpp:533
    #3 0x7fe10f0e7eeb in Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) /home/hsalo/src/exiv2/src/tiffimage.cpp:344
    #4 0x7fe10f0e59c4 in Exiv2::TiffImage::readMetadata() /home/hsalo/src/exiv2/src/tiffimage.cpp:187
    #5 0x563547182ad6 in Action::Print::printSummary() /home/hsalo/src/exiv2/src/actions.cpp:296
    #6 0x563547187077 in Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/hsalo/src/exiv2/src/actions.cpp:242
    #7 0x56354711d87b in main /home/hsalo/src/exiv2/src/exiv2.cpp:166
    #8 0x7fe10deaf2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
    #9 0x56354711e729 in _start (/home/hsalo/builds/exiv2/2018-01-08/bin/exiv2+0x12729)

0x60200000ed51 is located 0 bytes to the right of 1-byte region [0x60200000ed50,0x60200000ed51)
allocated by thread T0 here:
    #0 0x7fe10fd6ad70 in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc2d70)
    #1 0x7fe10ee5b358 in Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) /home/hsalo/src/exiv2/src/image.cpp:467
    #2 0x7fe10ee5d602 in Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) /home/hsalo/src/exiv2/src/image.cpp:533
    #3 0x7fe10f0e7eeb in Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) /home/hsalo/src/exiv2/src/tiffimage.cpp:344
    #4 0x7fe10f0e59c4 in Exiv2::TiffImage::readMetadata() /home/hsalo/src/exiv2/src/tiffimage.cpp:187
    #5 0x563547182ad6 in Action::Print::printSummary() /home/hsalo/src/exiv2/src/actions.cpp:296
    #6 0x563547187077 in Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/hsalo/src/exiv2/src/actions.cpp:242
    #7 0x56354711d87b in main /home/hsalo/src/exiv2/src/exiv2.cpp:166
    #8 0x7fe10deaf2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/hsalo/src/exiv2/src/iptc.cpp:354 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int)
Shadow bytes around the buggy address:
  0x0c047fff9d50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9da0: fa fa fa fa fa fa fa fa fa fa[01]fa fa fa fd fd
  0x0c047fff9db0: fa fa fd fa fa fa 00 fa fa fa 00 fa fa fa 00 fa
  0x0c047fff9dc0: fa fa 06 fa fa fa 00 04 fa fa 00 04 fa fa 00 04
  0x0c047fff9dd0: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04
  0x0c047fff9de0: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04
  0x0c047fff9df0: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa fd fa
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
xw48 commented 6 years ago

This has been assigned CVE-2017-17724

clanmills commented 6 years ago

This appears to be fixed on 'master'

506 rmills@rmillsmbp:~/Downloads $ exiv2 2018-01-09-exiv2-crash-002.tiff 
invalid type value detected in Image::printIFDStructure:  40448
Exiv2 exception in print action for file 2018-01-09-exiv2-crash-002.tiff:
invalid type value detected in Image::printIFDStructure
507 rmills@rmillsmbp:~/Downloads $
fgeek commented 6 years ago

@clanmills you need to build with ASan to detect the issue. Without ASan I see same output.

clanmills commented 6 years ago

Thanks, Henri. I've reproduced this and I am investigating.

ghost commented 6 years ago

Also can be triggered with Valgrind:

kbabioch@tumbleweed:~> valgrind exiv2 poc_1.tiff 
==10752== Memcheck, a memory error detector
==10752== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==10752== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==10752== Command: exiv2 poc_1.tiff
==10752== 
==10752== Conditional jump or move depends on uninitialised value(s)
==10752==    at 0x4F81200: Exiv2::Internal::binaryToString[abi:cxx11](unsigned char const*, unsigned long, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F812D8: Exiv2::Internal::binaryToString[abi:cxx11](Exiv2::DataBuf&, unsigned long, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F837BC: Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F853C4: Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEA303: Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEBD05: Exiv2::TiffImage::readMetadata() (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x127A08: Action::Print::printSummary() (in /usr/bin/exiv2)
==10752==    by 0x12964F: Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/bin/exiv2)
==10752==    by 0x113F0C: main (in /usr/bin/exiv2)
==10752== 
==10752== Invalid read of size 1
==10752==    at 0x4F88DA9: Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F83929: Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F853C4: Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEA303: Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEBD05: Exiv2::TiffImage::readMetadata() (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x127A08: Action::Print::printSummary() (in /usr/bin/exiv2)
==10752==    by 0x12964F: Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/bin/exiv2)
==10752==    by 0x113F0C: main (in /usr/bin/exiv2)
==10752==  Address 0x68a98c0 is 0 bytes after a block of size 0 alloc'd
==10752==    at 0x4C2EE1F: operator new[](unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==10752==    by 0x4F838F1: Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F853C4: Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEA303: Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEBD05: Exiv2::TiffImage::readMetadata() (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x127A08: Action::Print::printSummary() (in /usr/bin/exiv2)
==10752==    by 0x12964F: Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/bin/exiv2)
==10752==    by 0x113F0C: main (in /usr/bin/exiv2)
==10752== 
==10752== Invalid read of size 1
==10752==    at 0x4F88DB0: Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F83929: Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F853C4: Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEA303: Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEBD05: Exiv2::TiffImage::readMetadata() (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x127A08: Action::Print::printSummary() (in /usr/bin/exiv2)
==10752==    by 0x12964F: Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/bin/exiv2)
==10752==    by 0x113F0C: main (in /usr/bin/exiv2)
==10752==  Address 0x68a98c1 is 1 bytes after a block of size 0 alloc'd
==10752==    at 0x4C2EE1F: operator new[](unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==10752==    by 0x4F838F1: Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F853C4: Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEA303: Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEBD05: Exiv2::TiffImage::readMetadata() (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x127A08: Action::Print::printSummary() (in /usr/bin/exiv2)
==10752==    by 0x12964F: Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/bin/exiv2)
==10752==    by 0x113F0C: main (in /usr/bin/exiv2)
==10752== 
==10752== 
==10752== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==10752==  Access not within mapped region at address 0x6C6F000
==10752==    at 0x4F88DB0: Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F83929: Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4F853C4: Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEA303: Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x4FEBD05: Exiv2::TiffImage::readMetadata() (in /usr/lib64/libexiv2.so.26.0.0)
==10752==    by 0x127A08: Action::Print::printSummary() (in /usr/bin/exiv2)
==10752==    by 0x12964F: Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/bin/exiv2)
==10752==    by 0x113F0C: main (in /usr/bin/exiv2)
==10752==  If you believe this happened as a result of a stack
==10752==  overflow in your program's main thread (unlikely but
==10752==  possible), you can try to increase the size of the
==10752==  main thread stack using the --main-stacksize= flag.
==10752==  The main thread stack size used in this run was 8388608.
==10752== 
==10752== HEAP SUMMARY:
==10752==     in use at exit: 31,943 bytes in 491 blocks
==10752==   total heap usage: 860 allocs, 369 frees, 179,019 bytes allocated
==10752== 
==10752== LEAK SUMMARY:
==10752==    definitely lost: 0 bytes in 0 blocks
==10752==    indirectly lost: 0 bytes in 0 blocks
==10752==      possibly lost: 0 bytes in 0 blocks
==10752==    still reachable: 31,943 bytes in 491 blocks
==10752==         suppressed: 0 bytes in 0 blocks
==10752== Rerun with --leak-check=full to see details of leaked memory
==10752== 
==10752== For counts of detected and suppressed errors, rerun with: -v
==10752== Use --track-origins=yes to see where uninitialised values come from
==10752== ERROR SUMMARY: 3954513 errors from 3 contexts (suppressed: 0 from 0)
Speicherzugriffsfehler (Speicherabzug geschrieben)
clanmills commented 6 years ago

I know what's causing this and I'm working on a fix. The code in Exiv2::IptcData::printStructure is reading bytes past the end of buffer. The current code is:

    void IptcData::printStructure(std::ostream& out, const byte* bytes,const size_t size,uint32_t depth)
    {
        uint32_t i     = 0 ;
        while  ( i < size-3 && bytes[i] != 0x1c ) i++;
        ...

The following modification allows the "crashing tiff" (2018-01-09-exiv2-crash-002.tiff) to work:

    void IptcData::printStructure(std::ostream& out, const byte* bytes,const size_t size,uint32_t depth)
    {
        size_t    i = 0 ;
        while  ( (i+5) <  size && bytes[i] != 0x1c ) i++;
                if     ( (i+5) >= size ) return;
        ...

I will revisit the IPTC spec to remind myself about the data format of an IPTC buffer and how it is terminated.

clanmills commented 6 years ago

@kbabiochSUSE There is a secondary issue (as you've observed) with Exiv2::Internal::binaryToString(). By "secondary", I mean this not the principle reason for the crash in this bug report and my work-around ensures that binaryToString() isn't called.

However, you have made a valid observation that binaryToString() can cause issues and I'll investigate that once I've dealt with IptcData::printStructure().

clanmills commented 6 years ago

I think the "secondary" issue is already logged. https://github.com/Exiv2/exiv2/issues/209

The reason that it's "secondary" in this context is because Valgrind failed to detect the "primary" issue of the buffer overrun in Exiv2::IptcData::printStructure which was detected when clang compiled the code with -fsanitize=address

ret2libc commented 6 years ago

Is there a patch for this CVE-2017-17724?

D4N commented 6 years ago

@ret2libc #180 will fix this.

@fgeek The reproducer for this is exactly the same as the one for #209. Did you forget to upload a reproducer or am I missing a difference between the two reports?

D4N commented 6 years ago

This got fixed by #461.