Exiv2

Hanno

What are you doing here? This is another malformed file. It's very helpful to have those files, however I can't deal with an avalanche of illegal files at the moment. I'm in the end game of completing v0.26. The team will meet on Sunday to discuss RC1 v0.26: http://clanmills.com/exiv2/Exiv2v0-26RC1.pdf

How about proceeding as follows:

1) We change the title of #1247 to "Exceptions/Crashes due to malformed input files"
2) When you find another file, update #1247 with the file and symptoms.

Find all that you can and we'll deal with them in v0.27.

Are you aware of the term "fuzzing"? I believe this is the art of creating illegal files to put software under stress. I am aware that libexiv2 can be exploited in this way, however I have never had time to work on this topic. Images are read by the function readMetadata() in the image handlers src/tiffimage.cpp, src/jpgimage.cpp/etc. It's probably necessary to go through those functions line-by-line and devise files to break the code. Then engineer a fix in the code. And that won't be the end of the project - far from it. We can modify metadata/delete metadata which could also put the code under great stress. Such a project is probably a very considerable undertaking.

Dealing with this by dumping files on me one-at-a-time is very inefficient use of my time. The fix you provided earlier to #1247 was incorrect and generated compiler warnings on GCC. And I added your test file to our test harness. So, you've taken up 2 hours of my day. 2 hours that I would prefer to use on the bug hunt for v0.26 #1230.

If you'd like to take on the challenge of finding and fixing all of this, I will happily assist/mentor you. However I'd like to avoid a very discouraging avalanche of bug reports.

These files are actually a result of fuzzing. I'm using american fuzzy lop in combination with address sanitizer. See here [1] [2] for some docs.

If you feel it's more appropriate I can add them all to one bug.

[1] https://fuzzing-project.org/tutorial2.html
[2] https://fuzzing-project.org/tutorial3.html

Hanno

Thanks for doing this. I did more work on #1247 last night and again this morning (I'm in England). I've thought of a way to cure/hide this by adding a function Image::lint() which would have signal handlers. Image::lint() would "sniff" files by running readMetadata() and printStructure() in a safe context.

I have closed #1247. As you discover additional issues, please update this issue #1248. I'll add the Image::lint() function in v0.27 and this will make the code immune to crashes on opening files. Image::lint() will make the library safer. I will also look at small changes in readMetadata() and printStructure() on a case-by-case basis.

We are having a Team Meeting tomorrow at 13:00 UTC 2016-10-23. You are welcome to join us if you wish. http://dev.exiv2.org/boards/3/topics/2767

Robin

Thanks for working on this, Hanno. I've thought of a serious limitation with my idea of the function Image::lint(). It has to modify and restore signal handlers. If signal handlers are process wide, messing with them in a single function can never be thread-safe without a lock. I never want to add a hidden lock to libexiv2. None of this misery occurs with clang where the code throws an Exiv2 exception. The host application doesn't crash and can inform user that the file cannot be read.

I'm wondering if there is any point in working on this issue. It isn't an issue when you build with clang. The problem is 100% due to elderly compilers such as GCC and CL. It does not make sense to pollute the Exiv2 code with checks to remedy deficiencies in the build tools.

The idea/proposal to write Image::lint() can never be implemented in a thread safe manner. http://stackoverflow.com/questions/5282099/signal-handling-in-pthreads

I feel this issue and #1247 are "not a bug" and should be closed.

More proof that this is a build tools issue. I test last night's MSVC/2015 build. No problem:

C:\temp\dist\2015\x64\dll\Release\bin>exiv2 -vV | grep -e svn -e date -e time -e compiler
compiler=MSVC
date=Oct 23 2016
time=02:46:49
svn=4655
have_gmtime_r=0
have_timegm=0
xmlns=mediapro:http://ns.iview-multimedia.com/mediapro/1.0/

C:\temp\dist\2015\x64\dll\Release\bin>exiv2 -pa \Users\rmills\gnu\exiv2\trunk\test\data\exiv2-bug1247.jpg
Exiv2 exception in print action for file \Users\rmills\gnu\exiv2\trunk\test\data\exiv2-bug1247.jpg:
Not a valid ICC Profile

C:\temp\dist\2015\x64\dll\Release\bin>exiv2 -pR \Users\rmills\gnu\exiv2\trunk\test\data\exiv2-bug1247.jpg
STRUCTURE OF JPEG FILE: \Users\rmills\gnu\exiv2\trunk\test\data\exiv2-bug1247.jpg
 address | marker       |  length | data
       0 | 0xffd8 SOI
       2 | 0xffe2 APP2  |      16 | ICC_PROFILE.... chunk 0/0
      18 | 0xffffffffff É"ÿ∙Exiv2 exception in print action for file \Users\rmills\gnu\exiv2\trunk\test\data\exiv2-bug1247.jpg:
This does not look like a JPEG image

C:\temp\dist\2015\x64\dll\Release\bin>

1) Why are we working on this?
2) Is there any reason I should not close this issue?

I'm not sure why you come to the conclusion these are not bugs....

You say these don't affect clang, I can't reproduce that. All four issues also happen with clang (latest version 3.9.0, on Linux). I'm also not entirely sure if your comments are only related to the fpe issue or about all issues (but you asked me to put unrelated bugs into one bugreport...)

The floating point exception issues are "only" crashes, so I might understand you say that you don't want to fix them, but the buffer overflows are security issues. If it's not your intention to fix those then this is very problematic and at the very least you need to clearly state that exiv2 is not suitable for untrusted input.

I'm very unwilling to stick lots of code into Exiv2 to deal with illegal files. It will be a very considerable undertaking which might have unbounded scope. I'll build it with clang 3.9 and see what happens. If it throws an exception, I don't want to dig deeper.

I have to point out that I deal almost alone with Exiv2. I have to say "No" to some requests. I am not under any obligation to you to say that it is untrusted or anything else for that matter. If you had the courtesy to say "Thank You for investigating this Robin and being willing to commit 100 hours to this matter", I would feel more enthusiasm for your request. However you have been shown no appreciation for my cooperation.

One more comment. If you think this is a very important matter, the best way to have it resolved is for you to join the team and work on the issue.

You are correct. It does crash on clang 3.9 on Linux.

750 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ exiv2 -vVg compiler -g version -g date -g time
exiv2 0.25 001900 (64 bit build)
compiler=Clang
version=4.2.1 Compatible Clang 3.9.0 (tags/RELEASE_390/final)
date=Oct 31 2016
time=14:25:26
id=$Id: version.cpp 4590 2016-09-30 16:45:54Z robinwmills $
have_gmtime_r=1
have_timegm=1
xmlns=mediapro:http://ns.iview-multimedia.com/mediapro/1.0/
751 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ exiv2 -pa test/data/exiv2-bug1247.jpg
Exiv2 exception in print action for file test/data/exiv2-bug1247.jpg:
Not a valid ICC Profile
752 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ exiv2 -pR test/data/exiv2-bug1247.jpg
STRUCTURE OF JPEG FILE: test/data/exiv2-bug1247.jpg
 address | marker       |  length | data
       0 | 0xffd8 SOI  
       2 | 0xffe2 APP2  |      16 | ICC_PROFILE.... chunk 0/0
Segmentation fault (core dumped)
753 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ 

Apple Clang does not crash. It throws an exception. Here's the clang version info for the current Xcode (8.1 8B62) on Sierra 10.12.1

558 rmills@rmillsmbp:~/gnu/exiv2/trunk $ clang --version
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin16.1.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
559 rmills@rmillsmbp:~/gnu/exiv2/trunk $ 

I know nothing about hacking and exploiting buffer overflows. Perhaps we need more caution about them. This is a mind-boggling issue. The code to defend our image read/write code could be larger than the current library. It appears that Apple have generated code which catches the crash. We need more research on how to handle this within a library. And preferably a method that doesn't require us to add thousands of if statements.

Here is a tool that we could investigate. http://safecode.cs.illinois.edu/index.html It's a little elderly, however I found a September 2015 version: https://github.com/jtcriswell/safecode-llvm37/tree/master/cmake The LLVM web site promotes SAFECode as a current project http://llvm.org For sure that isn't working in clang 8.0 on MacOS-X:

579 rmills@rmillsmbp:~/gnu/exiv2/trunk $ clang --version
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin16.1.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
580 rmills@rmillsmbp:~/gnu/exiv2/trunk $ ./configure "CXXFLAGS=-g -fmemsafety" 
checking for g++... g++
checking whether the C++ compiler works... no
configure: error: in `/Users/rmills/gnu/exiv2/trunk':
configure: error: C++ compiler cannot create executables
See `config.log' for more details
581 rmills@rmillsmbp:~/gnu/exiv2/trunk $ 

827 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ which clang
/usr/bin/clang
828 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ clang --version
clang version 3.9.0-1ubuntu1 (tags/RELEASE_390/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
829 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $ ./configure "CXXFLAGS=-g -fmemsafety"checking for g++... g++
checking whether the C++ compiler works... no
configure: error: in `/home/rmills/gnu/exiv2/trunk':
configure: error: C++ compiler cannot create executables
See `config.log' for more details
830 rmills@rmillsmbp-ubuntu:~/gnu/exiv2/trunk $

Currently, we have an option bool bUseCurl.

    class EXIV2API ImageFactory {
    ...
    public:
    ...
        static BasicIo::AutoPtr createIo(const std::string& path, bool useCurl = true);
    ...
        static Image::AutoPtr open(const std::string& path, bool useCurl = true);
    ...
    };

Returning to Hanno's request that we say that Exiv2 is "not suitable for untrusted input". I will not do that. The default (for all open source software) is that the user must determine suitability for his/her purposes. We do not claim that Exiv2 is safe for all files. I doubt if we could ever make such a claim even if we pepper our code with checks for malicious images. I don't believe in 2016 that it is possible to guarantee the safety and reliability of any software.