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.

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

bad free in Exiv2::Image::~Image (image.cpp:173) #76

Closed
fantasy7082 opened this issue Sep 25, 2017 · 3 comments · Fixed by #79
Closed

bad free in Exiv2::Image::~Image (image.cpp:173) #76

fantasy7082 opened this issue Sep 25, 2017 · 3 comments · Fixed by #79
Milestone

Comments

@fantasy7082
Copy link

fantasy7082 commented Sep 25, 2017

I'm forwarding a security vulnerability reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=1495043

The file used to reproduce the issue is here:
https://bugzilla.redhat.com/attachment.cgi?id=1330345

Here's a copy of the report:

Liu Zhu 2017-09-24 23:04:22 EDT
Created attachment 1330345 [details]
poc_file

A bad free vulnerability was found in Exiv2::Image::~Image ,which allow attackers to cause a denial of service (bad free) via a crafted file.

./exiv2 -V
exiv2 0.26 001a00 (64 bit build)
Copyright (C) 2004-2017 Andreas Huggel.

./exiv2 010_bad_free
==49036==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x60300000d570 in thread T0
#0 0x7fbdb6de5b2a in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99b2a)
#1 0x7fbdb665c127 in std::pair<int const, std::__cxx11::basic_string<char, std::char_traits, std::allocator > >::~pair() /usr/include/c++/5/bits/stl_pair.h:96
....
.....
...
#14 0x7fbdb665b1a1 in std::map<int, std::__cxx11::basic_string<char, std::char_traits, std::allocator >, std::less, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits, std::allocator > > > >::~map() /usr/include/c++/5/bits/stl_map.h:96
#15 0x7fbdb665310b in Exiv2::Image::~Image() /root/fuzzing/exiv2/src/image.cpp:173
#16 0x7fbdb66c30a5 in Exiv2::TiffImage::~TiffImage() /root/fuzzing/exiv2/include/exiv2/tiffimage.hpp:60
#17 0x7fbdb66c310d in Exiv2::TiffImage::~TiffImage() /root/fuzzing/exiv2/include/exiv2/tiffimage.hpp:60
#18 0x455263 in std::auto_ptrExiv2::Image::~auto_ptr() /usr/include/c++/5/backward/auto_ptr.h:170
#19 0x43ce5e in Action::Print::printSummary() /root/fuzzing/exiv2/src/actions.cpp:287
#20 0x43974b in Action::Print::run(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) /root/fuzzing/exiv2/src/actions.cpp:244
#21 0x421fb9 in main /root/fuzzing/exiv2/src/exiv2.cpp:170
#22 0x7fbdb598b82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#23 0x4219b8 in _start (/usr/local/exiv2_ASAN/bin/exiv2+0x4219b8)

Other security vulnerability reports(2017-09-22) are here:
https://bugzilla.redhat.com/show_bug.cgi?id=1494443
https://bugzilla.redhat.com/show_bug.cgi?id=1494778
https://bugzilla.redhat.com/show_bug.cgi?id=1494781
https://bugzilla.redhat.com/show_bug.cgi?id=1494782
https://bugzilla.redhat.com/show_bug.cgi?id=1494787

@clanmills
Copy link
Collaborator

clanmills commented Sep 25, 2017

Thank You for these fuzzing reports. They will be investigated.

May I ask about your intentions here? You are aware that all of those reports are public and paint Exiv2 in a poor light. Do you intend to submit 100s of similar reports which will probably result in the death of Exiv2. Can you make this stuff less public, or is your intention to kill all the work that I have put into Exiv2 over 10 years?

Team Exiv2 is effectively one person (me). I do get help from other contributors, however not on a sustained day-in-day-out basis. There are many people making requests and I try to deal with them quickly and effectively. However when you are posting 10 vulnerabilities every day, you are building my TODO list more quickly than I handle. I am about to set off on a 9 week journey and will have limited time for Exiv2 while away from home.

Are you hostile or helpful?

@fantasy7082
Copy link
Author

I'm sorry my report has bothered you. Please be assured that I'm not hostile. I'm just an analyst for software security vulnerabilities. If you don't feel so good, I'll delete the information from GitHub.

@fantasy7082 fantasy7082 changed the title bad free in Exiv2::Image::~Image (image.cpp:173) bad free in Exiv2::Image... Sep 25, 2017
@fantasy7082 fantasy7082 changed the title bad free in Exiv2::Image... bad free in Exiv2::Image::~Image (image.cpp:173) Sep 25, 2017
@clanmills
Copy link
Collaborator

clanmills commented Sep 25, 2017

Leave the reports. They will be dealt with. However, I ask you to stop posting more on GitHub and Exiv2's Redmine site. To be sure of my attention, you are welcome to post a "digest" of newly discovered vulnerabilities about once a month.

Last year, I started thinking about fuzzing and put that as the top priority item for v0.27. I didn't say "security" as I didn't want to alarm users about my true thoughts. http://dev.exiv2.org/news/3

In July I received a hostile email pointing me to a discussion on a forum. Somebody even said "Exiv2 must be rewritten". Exactly how rewriting all the metadata code in Exiv2 would improve the security is not explained. And, the author of that opinion, offered no suggestion about how to find engineers to undertake the effort which would be about 20 man-years of work.

I accept that security is a very important matter. Somebody said that image vulnerabilities are now a favourite vector of platform attack. This vector is directed against the platform and we need a community response. I have made two suggestions:

  1. We need an "image lint" library that can quickly assess an image as "safe".
  2. The Linux kernel should quarantine images until they have been assessed as "safe".

My proposals were bluntly rejected and I was told that it is the responsibility of Exiv2 to never crash. And presumably all code that parses images is being told to do this. Exiv2 is fast. On my laptop, we read about 1000 images/second. With some effort, Exiv2 could be "libimagelint" and quickly say that an image is safe or dangerous.

However, the response of the community has been to say that my ideas are not good. No discussion.

As we agreed about the vulnerability, we should agree to a community response to the threat. Perhaps we should have a conference to talk together and invite parties to discuss how best to proceed. The current strategy adopted by the community to bombard me with bug reports will have three consequences:

  1. You will undermine all public confidence in Exiv2.
  2. I will leave the community.
  3. The platform will have no supported C++ Metadata Library.

As I work on my own at home in England, I don't know who asked you to undertake your work. Perhaps you could report my thoughts to others. I am very willing to discuss this serious matter and assure you of my attention.

D4N added a commit to D4N/exiv2 that referenced this issue Sep 27, 2017
=> Should fix Exiv2#76 (most of the work has been done by Robin Mills in
   6e3855a)

The problem with Exiv2#76 is the contents of the 26th IFD, with the
following contents:
tag: 0x8649
type: 0x1
count: 0xffff ffff
offset: 0x4974

The issue is the size of count (uint32_t), as adding anything to it
causes an overflow. Especially the expression:
(size*count + pad+20)
results in an overflow and gives 20 as a result instead of
0x100000014, thus the condition in the if in the next line is false
and the program continues to run (until it crashes at io.read).

To properly account for the overflow, the brackets have to be removed,
as then the result is saved in the correctly sized type and not cast
after being calculated in the smaller type.

The brackets have also been removed from bigtiffimage.cpp, where the
same issue is present.
D4N added a commit to D4N/exiv2 that referenced this issue Sep 28, 2017
=> Should fix Exiv2#76 (most of the work has been done by Robin Mills in
   6e3855a)

The problem with Exiv2#76 is the contents of the 26th IFD, with the
following contents:
tag: 0x8649
type: 0x1
count: 0xffff ffff
offset: 0x4974

The issue is the size of count (uint32_t), as adding anything to it
causes an overflow. Especially the expression:
(size*count + pad+20)
results in an overflow and gives 20 as a result instead of
0x100000014, thus the condition in the if in the next line is false
and the program continues to run (until it crashes at io.read).

To properly account for the overflow, the brackets have to be removed,
as then the result is saved in the correctly sized type and not cast
after being calculated in the smaller type.

The brackets have also been removed from bigtiffimage.cpp, where the
same issue is present.
D4N added a commit to D4N/exiv2 that referenced this issue Oct 15, 2017
=> Should fix Exiv2#76 (most of the work has been done by Robin Mills in
   6e3855a)

The problem with Exiv2#76 is the contents of the 26th IFD, with the
following contents:
tag: 0x8649
type: 0x1
count: 0xffff ffff
offset: 0x4974

The issue is the size of count (uint32_t), as adding anything to it
causes an overflow. Especially the expression:
(size*count + pad+20)
results in an overflow and gives 20 as a result instead of
0x100000014, thus the condition in the if in the next line is false
and the program continues to run (until it crashes at io.read).

To properly account for the overflow, the brackets have to be removed,
as then the result is saved in the correctly sized type and not cast
after being calculated in the smaller type.
D4N added a commit to D4N/exiv2 that referenced this issue Oct 18, 2017
D4N added a commit that referenced this issue Oct 29, 2017
Added reproducer for #76 /  CVE-2017-14857 to the testsuite
@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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants