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

Segmentation fault caused by uncontrolled recursion of Exiv2::Image::printIFDStructure (src/image.cpp) #216

Closed
ProbeFuzzer opened this issue Jan 18, 2018 · 3 comments · Fixed by #479
Assignees
Labels
Projects
Milestone

Comments

@ProbeFuzzer
Copy link

ProbeFuzzer commented Jan 18, 2018

On latest version of exiv2 (0.26) and the latest master branch:
there is a segmentation fault caused by uncontrolled recursion of Exiv2::Image::printIFDStructure function in src/image.cpp file, which could cause a denial of service via a crafted tif file.

This issue could be reproduced by command: exiv2 $POC.
POC is available at: https://github.com/ProbeFuzzer/poc/blob/master/exiv2/exiv2_0-26_exiv2_uncontrolled-recursion_printIFDStructure.tif

The stack trace is as follows:

    ...Recursion of  Exiv2::Image::printIFDStructure call...
    at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/image.cpp:468
#20679 0x00007ffff7b9d6c2 in Exiv2::Image::printIFDStructure (this=0x6635b0, io=..., out=..., option=Exiv2::kpsRecursive, start=8, bSwap=true, c=77 'M', depth=2)
    at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/image.cpp:468
#20680 0x00007ffff7b9d6c2 in Exiv2::Image::printIFDStructure (this=0x6635b0, io=..., out=..., option=Exiv2::kpsRecursive, start=8, bSwap=true, c=77 'M', depth=1)
    at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/image.cpp:468
#20681 0x00007ffff7b9d6c2 in Exiv2::Image::printIFDStructure (this=0x6635b0, io=..., out=..., option=Exiv2::kpsRecursive, start=8, bSwap=true, c=77 'M', depth=0)
    at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/image.cpp:468
#20682 0x00007ffff7b9df98 in Exiv2::Image::printTiffStructure (this=0x6635b0, io=..., out=..., option=Exiv2::kpsRecursive, depth=-1, offset=0)
    at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/image.cpp:547
#20683 0x00007ffff7be882b in Exiv2::TiffImage::printStructure (this=0x6635b0, out=..., option=Exiv2::kpsRecursive, depth=0) at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/tiffimage.cpp:346
#20684 0x00007ffff7be7948 in Exiv2::TiffImage::readMetadata (this=0x6635b0) at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/tiffimage.cpp:188
#20685 0x000000000042adde in Action::Print::printSummary (this=0x662210) at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/actions.cpp:297
#20686 0x000000000042a9b3 in Action::Print::run (this=0x662210, path=...) at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/actions.cpp:243
#20687 0x000000000041bfb9 in main (argc=2, argv=0x7fffffffdd88) at /u/youwei/ProbeFuzzer/product/exiv2/patch/src/src/exiv2.cpp:166
@clanmills clanmills self-assigned this Jan 18, 2018
@clanmills
Copy link
Collaborator

Thanks. Today's my birthday. Thank you for this present!

Interesting file. You've inserted an IFD that "points to" the IFD at the top of the file. Ingeneous trap:

743 rmills@rmillsmbp:~/gnu/github/exiv2/exiv2 $ build/bin/exiv2 -pR ~/temp/exiv2_0-26_exiv2_uncontrolled-recursion_printIFDStructure.tif 
STRUCTURE OF TIFF FILE (MM): /Users/rmills/temp/exiv2_0-26_exiv2_uncontrolled-recursion_printIFDStructure.tif
 address |    tag                              |      type |    count |    offset | value
      10 | 0x0100 ImageWidth                   |     SHORT |        1 |           | 1
      22 | 0x0103 Compression                  |     SHORT |        0 |           | 
      34 | 000000 GPSVersionID                 |      BYTE |        0 |           | 
      46 | 000000 GPSVersionID                 |       IFD |        8 |         4 |  ...
  STRUCTURE OF TIFF FILE (MM): /Users/rmills/temp/exiv2_0-26_exiv2_uncontrolled-recursion_printIFDStructure.tif
   address |    tag                              |      type |    count |    offset | value
        10 | 0x0100 ImageWidth                   |     SHORT |        1 |           | 1
        22 | 0x0103 Compression                  |     SHORT |        0 |           | 
        34 | 000000 GPSVersionID                 |      BYTE |        0 |           | 
        46 | 000000 GPSVersionID                 |       IFD |        8 |         4 |  ...

The "quick fix" is to test for unreasonably deep recursion and throw:

    void Image::printIFDStructure(BasicIo& io, std::ostream& out, Exiv2::PrintStructureOption option,uint32_t start,bool bSwap,char c,int depth)
    {
        depth++;
        bool bFirst  = true  ;
        if ( depth > 20 ) { // https://github.com/Exiv2/exiv2/issues/216
            throw Error(58);
        }

I'll think about this for a day or two and submit a fix.

@clanmills
Copy link
Collaborator

I've done some additional work on this. We have an outstanding PR to remove printIFDStructure() from readMetadata(). #180

I've provided a patch for tiffimage.cpp (and other image types) which I believe will allow that PR to be submitted. As that removes the call to printStructure() from readMetadata(), this crash will also disappear.

However printStructure() is a useful debugging function and I would like to retain it in the code base at the moment. The "quick fix" mentioned above:

if ( depth > 20 ) { throw Error(58); }

should be added to the the code to prevent an infinite recursive loop which can be caused by the command $ exiv2 -pR exiv2_0-26_exiv2_uncontrolled-recursion_printIFDStructure.tif

@ProbeFuzzer
Copy link
Author

@clanmills, Happy birthday to you! Thanks a lot for your contribution to the exiv2 community.

@piponazo piponazo added the bug label May 27, 2018
@piponazo piponazo added this to TODO in v0.27 May 29, 2018
D4N added a commit that referenced this issue Aug 30, 2018
D4N added a commit that referenced this issue Sep 1, 2018
D4N added a commit that referenced this issue Sep 1, 2018
piponazo pushed a commit that referenced this issue Sep 10, 2018
D4N added a commit that referenced this issue Sep 10, 2018
D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018
The bug got resolved by PR Exiv2#461 (slices).
@D4N D4N moved this from TODO to In Progress in v0.27 Oct 11, 2018
D4N added a commit to D4N/exiv2 that referenced this issue Oct 11, 2018
The bug 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 D4N moved this from In Progress to Done in v0.27 Oct 12, 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
Labels
Projects
No open projects
v0.27
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants