New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 opened this Issue Jan 9, 2018 · 11 comments

Comments

Projects
6 participants
@fgeek
Collaborator

fgeek commented Jan 9, 2018

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

==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

This comment has been minimized.

xw48 commented Feb 11, 2018

This has been assigned CVE-2017-17724

@clanmills

This comment has been minimized.

Collaborator

clanmills commented Feb 11, 2018

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

This comment has been minimized.

Collaborator

fgeek commented Feb 13, 2018

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

@clanmills clanmills self-assigned this Feb 13, 2018

@clanmills

This comment has been minimized.

Collaborator

clanmills commented Feb 13, 2018

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

@ghost

This comment has been minimized.

ghost commented Feb 13, 2018

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

This comment has been minimized.

Collaborator

clanmills commented Feb 13, 2018

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

This comment has been minimized.

Collaborator

clanmills commented Feb 13, 2018

@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

This comment has been minimized.

Collaborator

clanmills commented Feb 13, 2018

I think the "secondary" issue is already logged. #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

@D4N D4N changed the title from heap-buffer-overflow src/iptc.cpp:354 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) to CVE-2017-17724: heap-buffer-overflow src/iptc.cpp:354 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) Feb 19, 2018

@ret2libc

This comment has been minimized.

ret2libc commented Apr 16, 2018

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

@D4N

This comment has been minimized.

Contributor

D4N commented Aug 30, 2018

@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 added a commit to D4N/exiv2 that referenced this issue Oct 2, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 3, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 3, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 3, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 3, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 3, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 3, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 4, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 4, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 4, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 10, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018

D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018

@D4N

This comment has been minimized.

Contributor

D4N commented Oct 11, 2018

This got fixed by #461.

@D4N D4N closed this Oct 11, 2018

@D4N D4N moved this from TODO to Done in v0.27 Oct 11, 2018

@clanmills clanmills added this to the v0.27 milestone Nov 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment