Skip to content
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.

Sign up for GitHub

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

Jump to bottom

heap-buffer-overflow in Exiv2::IptcData::printStructure #159

Closed
gy741 opened this issue Nov 10, 2017 · 6 comments · Fixed by #160 or #479
Closed

heap-buffer-overflow in Exiv2::IptcData::printStructure #159

gy741 opened this issue Nov 10, 2017 · 6 comments · Fixed by #160 or #479
Assignees
@piponazo @D4N
Labels
bug
Milestone
v0.27

Comments

@gy741
Copy link

gy741 commented Nov 10, 2017

Hello.

I found a heap-buffer-overflow bug in exiv2.

Please confirm.

Thanks.

OS: Ubuntu 16.04 64bit
Steps to reproduce:
1.Download the .POC files.
2.Compile the source code with ASan.
PoC download : PoC

=================================================================
==31776==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000e951 at pc 0x7f5a274c7667 bp 0x7ffd77997f30 sp 0x7ffd77997f28
READ of size 1 at 0x60200000e951 thread T0
    #0 0x7f5a274c7666 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) /root/karas/exiv2/src/iptc.cpp:359:26
    #1 0x7f5a274a1516 in Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) /root/karas/exiv2/src/image.cpp:467:25
    #2 0x7f5a274a7090 in Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) /root/karas/exiv2/src/image.cpp:530:13
    #3 0x7f5a2769428f in Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) /root/karas/exiv2/src/tiffimage.cpp:348:9
    #4 0x7f5a2768bf63 in Exiv2::TiffImage::readMetadata() /root/karas/exiv2/src/tiffimage.cpp:191:9
    #5 0x525002 in Action::Print::printSummary() /root/karas/exiv2/src/actions.cpp:292:9
    #6 0x524758 in Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /root/karas/exiv2/src/actions.cpp:244:44
    #7 0x4f71a7 in main /root/karas/exiv2/src/exiv2.cpp:170:19
    #8 0x7f5a25b6d82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #9 0x422418 in _start (/root/karas/exiv2/build/bin/exiv2+0x422418)

0x60200000e951 is located 0 bytes to the right of 1-byte region [0x60200000e950,0x60200000e951)
allocated by thread T0 here:
    #0 0x4f3c90 in operator new[](unsigned long) (/root/karas/exiv2/build/bin/exiv2+0x4f3c90)
    #1 0x7f5a274a13fc in Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) /root/karas/exiv2/src/image.cpp:464:37
    #2 0x7f5a274a7090 in Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) /root/karas/exiv2/src/image.cpp:530:13

SUMMARY: AddressSanitizer: heap-buffer-overflow /root/karas/exiv2/src/iptc.cpp:359:26 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int)
Shadow bytes around the buggy address:
  0x0c047fff9cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9d20: fa fa fa fa fa fa fa fa fa fa[01]fa fa fa fd fd
  0x0c047fff9d30: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d40: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d50: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d60: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d70: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
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
==31776==ABORTING
D4N added a commit to D4N/exiv2 that referenced this issue Nov 10, 2017
@D4N
Fixed potential out of bounds file access
36df4bc 
This commit adds a out-of-bounds protection in the case that the
extracted values for offset & count are summed up larger than the size
of the file. Also this function checks for overflows before performing
the addition.

This fixes Exiv2#159
@D4N D4N mentioned this issue Nov 10, 2017
Merged
@D4N
Copy link
Member

D4N commented Nov 10, 2017

Thanks for the report, the issue should be fixed by #160.

@D4N D4N closed this as completed in #160 Nov 10, 2017
@gy741
Copy link
Author

gy741 commented Nov 11, 2017

@D4N

Hello.

I checked the patch.

But, I found New bug.

Please confirm.

Thanks.

PoC download : PoC

=================================================================
==15673==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000e951 at pc 0x7f10afe85977 bp 0x7ffd0d96cbf0 sp 0x7ffd0d96cbe8
READ of size 1 at 0x60200000e951 thread T0
    #0 0x7f10afe85976 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int) /root/karas/exiv2/src/iptc.cpp:359:26
    #1 0x7f10afe5f5dd in Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) /root/karas/exiv2/src/image.cpp:474:25
    #2 0x7f10afe653a0 in Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) /root/karas/exiv2/src/image.cpp:537:13
    #3 0x7f10b005257f in Exiv2::TiffImage::printStructure(std::ostream&, Exiv2::PrintStructureOption, int) /root/karas/exiv2/src/tiffimage.cpp:348:9
    #4 0x7f10b004a253 in Exiv2::TiffImage::readMetadata() /root/karas/exiv2/src/tiffimage.cpp:191:9
    #5 0x525002 in Action::Print::printSummary() /root/karas/exiv2/src/actions.cpp:292:9
    #6 0x524758 in Action::Print::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /root/karas/exiv2/src/actions.cpp:244:44
    #7 0x4f71a7 in main /root/karas/exiv2/src/exiv2.cpp:170:19
    #8 0x7f10ae52b82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #9 0x422418 in _start (/root/karas/exiv2/build/bin/exiv2+0x422418)

0x60200000e951 is located 0 bytes to the right of 1-byte region [0x60200000e950,0x60200000e951)
allocated by thread T0 here:
    #0 0x4f3c90 in operator new[](unsigned long) (/root/karas/exiv2/build/bin/exiv2+0x4f3c90)
    #1 0x7f10afe5f4c8 in Exiv2::Image::printIFDStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, unsigned int, bool, char, int) /root/karas/exiv2/src/image.cpp:471:37
    #2 0x7f10afe653a0 in Exiv2::Image::printTiffStructure(Exiv2::BasicIo&, std::ostream&, Exiv2::PrintStructureOption, int, unsigned long) /root/karas/exiv2/src/image.cpp:537:13

SUMMARY: AddressSanitizer: heap-buffer-overflow /root/karas/exiv2/src/iptc.cpp:359:26 in Exiv2::IptcData::printStructure(std::ostream&, unsigned char const*, unsigned long, unsigned int)
Shadow bytes around the buggy address:
  0x0c047fff9cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9d20: fa fa fa fa fa fa fa fa fa fa[01]fa fa fa fd fd
  0x0c047fff9d30: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d40: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d50: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d60: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9d70: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
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
==15673==ABORTING

@clanmills clanmills self-assigned this Nov 11, 2017
@clanmills
Copy link
Collaborator

Can you provide the image in another format, please. I don't know how to decompress a .DMS file on the Mac.

@clanmills clanmills reopened this Nov 11, 2017
@clanmills
Copy link
Collaborator

clanmills commented Nov 15, 2017 •
edited
Loading

The file printStructure.dms is an image (it's not a compressed image). The PR disable-printStructure fixes this. However, I haven't added printStructure.dms to the test suite.

I'm 100% defeated by Git and PR disable-printStructure_ which appears to include the totally unrelated PR documentation-update. The PR disable-printStructure breaks the test suite and although I know how to fix the suite on my local machine, I don't know how to submit my fix to master.

@clanmills clanmills assigned piponazo and D4N and unassigned clanmills Nov 15, 2017
@clanmills
Copy link
Collaborator

@D4N and @piponazo

Can I ask you to unravel the mess with this issue, PR disable-printStructure and PR documentation-update I feel totally locked out of Exiv2 by GIT. I'm unable to submit anything to the system.

Alison and I are off to weddings in Scotland and Vietnam tomorrow (2017-11-16). We get home on 2017-12-05. I'll answer email while travelling. I'm not going to attempt to interact with GIT. Assuming that disable-printStructure has the effect of stabilising the security of Exiv2, I'll be happy to work in December on publishing v0.27. The website was moved to SVN in September and I can make the release without touching Git.

And then it is time for me to retire. In 2018 I am willing to continue answering user issues about Exiv2 and to help you in any way with the project via email or Slack. However, I am defeated by Git and locked out of http://github.com/exiv2/exiv2

@piponazo
Copy link
Collaborator

Hi @gy741 , I was spending some time checking out all the open issues in the project and you are right. The second POC you provided is still making exvi2 crash (when using the sanitizers). However the issue with the first POC is solved.

Our plan is to stop using the printStructure function soon. We'll keep you updated.

D4N added a commit that referenced this issue Aug 30, 2018
@D4N
Add second reproducer for #159 to the test suite
3361008 
This fixes #159
D4N added a commit that referenced this issue Sep 1, 2018
@D4N
Add second reproducer for #159 to the test suite
e876488 
This fixes #159
D4N added a commit that referenced this issue Sep 1, 2018
@D4N
Add second reproducer for #159 to the test suite
f3e3bf3 
This fixes #159
piponazo pushed a commit that referenced this issue Sep 10, 2018
@D4N @piponazo
Add second reproducer for #159 to the test suite
f1d47d7 
This fixes #159
D4N added a commit that referenced this issue Sep 10, 2018
@D4N
Add second reproducer for #159 to the test suite
4fab4ed 
This fixes #159
a17r pushed a commit to a17r/exiv2 that referenced this issue Sep 18, 2018
@D4N @a17r
Fixed potential out of bounds file access
4e9cc37 
This commit adds a out-of-bounds protection in the case that the
extracted values for offset & count are summed up larger than the size
of the file. Also this function checks for overflows before performing
the addition.

This fixes Exiv2#159
D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018
@D4N
[testsuite] Add reproducer for second bug from Exiv2#159
70437ad 
The bug described in the issue got resolved by PR Exiv2#461 (slices).
@D4N D4N added the bug label Oct 11, 2018
@D4N D4N mentioned this issue Oct 11, 2018
Merged
D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018
@D4N
[testsuite] Add reproducer for second bug from Exiv2#159
eeb520b 
The bug described in the issue got resolved by PR Exiv2#461 (slices).
@D4N D4N closed this as completed in #479 Oct 12, 2018
D4N added a commit that referenced this issue Oct 12, 2018
@D4N
Merge pull request #479 from D4N/add_reproducers
d68f42a 
Add reproducers for #159, #216 and #263
@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
Assignees

@piponazo piponazo

@D4N D4N

Labels
bug
Projects
None yet
Milestone
v0.27
4 participants
@piponazo @clanmills @D4N @gy741