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

exiv2 -pR false negative "tiff directory length is too large" #1805

Closed
clanmills opened this issue Jul 28, 2021 · 7 comments · Fixed by #1929
Closed

exiv2 -pR false negative "tiff directory length is too large" #1805

clanmills opened this issue Jul 28, 2021 · 7 comments · Fixed by #1929
Assignees
Milestone

Comments

@clanmills
Copy link
Collaborator

$ tvisitor -pRU path is more-or-less the same as $ exiv2 -pR path and the file is good.

It must be a bug in $ exiv2 -pR and almost certainly caused by reading dirLength from the wrong offset into the file. So, it's a bug in $ exiv2 -pR path. This is a false negative.

DSC00723

@clanmills clanmills added this to the v1.00 milestone Jul 28, 2021
@clanmills clanmills self-assigned this Jul 28, 2021
@postscript-dev
Copy link
Collaborator

I remembered that when I added the Exif.SonyMisc3c.* tags, I included one of Exiftool's test images which is the same model number as the above image (Sony DSC-HX60V). I tried running exiv2 -pR and this does trigger the error.

@clanmills
Copy link
Collaborator Author

@postscript-dev and @hassec Can you test this patch, please. If you're happy, a PR should be prepared with test changes.

Please be aware that the option -pR is only supported in Debug builds. There's magic required in the test suite to safely skip this test for non Debug builds (see tests/bugfixes/github/test_issue_216.py).

diff --git a/src/image.cpp b/src/image.cpp
index e5a182c2..9436b91a 100644
--- a/src/image.cpp
+++ b/src/image.cpp
@@ -490,7 +490,10 @@ namespace Exiv2 {
                         seekOrThrow(io, offset, BasicIo::beg, kerCorruptedMetadata);  // position
                         readOrThrow(io, bytes, jump, kerCorruptedMetadata)     ;  // read
                         bytes[jump]=0               ;
-                        if ( ::strcmp("Nikon",chars) == 0 ) {
+                        bool bNikon = ::strcmp("Nikon"    ,chars) == 0;
+                        bool bSony  = ::strcmp("SONY DSC ",chars) == 0;
+                        
+                        if ( bNikon ) {
                             // tag is an embedded tiff
                             const long byteslen = count-jump;
                             DataBuf bytes(byteslen);  // allocate a buffer
@@ -499,8 +502,9 @@ namespace Exiv2 {
                             printTiffStructure(memIo,out,option,depth);
                         } else {
                             // tag is an IFD
+                            uint32_t punt = bSony ? 12 : 0 ;
                             seekOrThrow(io, 0, BasicIo::beg, kerCorruptedMetadata);  // position
-                            printIFDStructure(io,out,option,offset,bSwap,c,depth);
+                            printIFDStructure(io,out,option,offset+punt,bSwap,c,depth);
                         }
 
                         seekOrThrow(io, restore, BasicIo::beg, kerCorruptedMetadata); // restore

@postscript-dev
Copy link
Collaborator

I have downloaded the patch, compiled and wrote a Python script to process all of Exiftool's Sony sample images.
My Python script is

import os

files = os.listdir()
    
for filename in files:
    os.system("""exiv2 -pR """ + filename)

stdout = [""]
stderr = [""]
retval = [0]

I ran this with

$ python3 exiv2_pR.py > exiv2_pR_output_std.txt 2> exiv2_pR_output_stderr.txt

The stderr outputs of before and after the patch are here:

  1. exiv2_pR_before_stderr.txt
  2. exiv2_pR_after_stderr.txt

The patch works well on the test image I supplied and some of the other DSC jpegs, but the other images in the collection now have new errors. As interesting one is the Uncaught exception: Begin must be smaller than end which doesn't output the name of the file, but the name can be tracked down in the alphebetical list of files as one that is missing.

Thanks for taking the time to look into this.

@clanmills
Copy link
Collaborator Author

Your python script is approximately what you'll need.

You should cut'n'paste into a script in tests/bugfixes/github to uses the test framework. I think @hassec can do that in the PR.

@clanmills
Copy link
Collaborator Author

clanmills commented Jul 30, 2021

I've looked at the 739 files in the ExifTool-Sony archive. Two are odd and cause tvisitor to hang. They are:

697 rmills@rmillsm1:~/Downloads/Sony $ dir odd
-rwxr-xr-x@ 1 rmills  staff   432B 14 Dec  2010 ./odd/SonyDCR-DVD905.jpg*
-rwxr-xr-x@ 1 rmills  staff   8.7K 10 Jun  2010 ./odd/SonyDCR-DVD708E.jpg*
698 rmills@rmillsm1:~/Downloads/Sony $ 

The others run successfully in tvisitor. However $ exiv2 -pR path, reports 82 false negatives. These are probably easy to fix. Some Sony MakerNotes start with the header SONY DSC , and clearly some do not.

The two odd files will require additional investigation.

I'll investigate and update the patch.

@clanmills
Copy link
Collaborator Author

The MakerNote for some Sony files has a header "SONY DSC " and others "SONY CAM ". The quick change to the patch is:

bool bSony  = ::strncmp("SONY ",chars,5) == 0;

The odd files have are illegal files for which $ exiv2 -pR path correctly throws an exception. The command $ exiv2 -pa path dangerously/wrongly forgives and reports a warning.

@postscript-dev
Copy link
Collaborator

Sorry, but it will be next week when I have time to look at this. I spent more time working on the manpage than I originally planned.

postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Sep 26, 2021
`exiv2 -pR` failed to process Sony makernotes. Fix checks if
makernotes header has Sony string, then uses correct offset.

Fix supplied by @clanmills
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Sep 26, 2021
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Sep 26, 2021
`exiv2 -pR` failed to process Sony makernotes. Fix checks if
makernotes header has Sony string, then uses correct offset.

Fix supplied by @clanmills
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Sep 26, 2021
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Oct 15, 2021
`exiv2 -pR` failed to process Sony makernotes. Fix checks if
makernotes header has Sony string, then uses correct offset.

Fix supplied by @clanmills
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Oct 15, 2021
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Nov 2, 2021
`exiv2 -pR` failed to process Sony makernotes. Fix checks if
makernotes header has Sony string, then uses correct offset.

Fix supplied by @clanmills
postscript-dev added a commit to postscript-dev/exiv2 that referenced this issue Nov 2, 2021
postscript-dev added a commit that referenced this issue Jan 9, 2022
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.

3 participants