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 occasionally crashes #2001

Closed
hansph opened this issue Nov 21, 2021 · 19 comments · Fixed by #2015
Closed

exiv2 occasionally crashes #2001

hansph opened this issue Nov 21, 2021 · 19 comments · Fixed by #2015
Assignees

Comments

@hansph
Copy link

hansph commented Nov 21, 2021

In some of my JPGs, exiv2 crashes with
Exiv2 exception in print action for file DSC01235.JPG: corrupted image metadata
exiftool can happily read the meta data.

This happens on a recent Manjaro Stable and was noticed first from Digikam. Exiv2 version is 0.27.5. The previous Manjaro release did not have this issue.

Example is too large to attach, see here: https://c.web.de/@321912251520915791/qxOFi7fXQQSE3TVnv2-08g

HP

@clanmills
Copy link
Collaborator

I am unable to download your test file. Can't investigate this without a test image.

@hansph
Copy link
Author

hansph commented Nov 21, 2021

Sorry, my fault, above URL is wrong, this hopefully works:
test image

@clanmills
Copy link
Collaborator

Third time, lucky. Your link hasn't changed! https://c.web.de/@321912251520915791/qxOFi7fXQQSE3TVnv2-08g

@hansph
Copy link
Author

hansph commented Nov 21, 2021 via email

@clanmills
Copy link
Collaborator

Thanks. I have the file now, thank you. I have reproduced the "corrupted image metadata" error.

I will investigate tomorrow.

@clanmills
Copy link
Collaborator

I understand the issue. It's been introduced by #1881. Sony files can store large previews in the MakerNote. In your case, its 1072653 bytes. I proposed the change to limit this to 1024*1024 bytes. The quick fix is to increase this to 8mb.

diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp
index 9c472b51..f608e8c2 100644
--- a/src/tiffvisitor_int.cpp
+++ b/src/tiffvisitor_int.cpp
@@ -1633,7 +1633,7 @@ namespace Exiv2 {
             v->read(pData, size, byteOrder());
         } else {
             // Prevent large memory allocations: https://github.com/Exiv2/exiv2/issues/1881
-            enforce(isize <= 1024 * 1024, kerCorruptedMetadata);
+            enforce(isize <= 8*1024 * 1024, kerCorruptedMetadata);
 
             // #1143 Write a "hollow" buffer for the preview image
             //       Sadly: we don't know the exact location of the image in the source (it's near offset)

A better strategy is to issue a warning and skip tag 0x2001 when it's more than 1mb.

602 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ env DYLD_LIBRARY_PATH=/Users/rmills/gnu/github/exiv2/0.27-maintenance/build/lib/ bin/exiv2 ~/Downloads/DSC01235.JPG 
Error: Directory Sony1, entry 0x2001 has invalid size 1072653*1; skipping entry.
File name       : /Users/rmills/Downloads/DSC01235.JPG
File size       : 20185088 Bytes
MIME type       : image/jpeg
...

This fix is incomplete as we must modify /tests/bugfixes/github/test_issue_1881.py as exiv2 will warns instead of throwing an exception.

diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp
index 9c472b51..62e0b050 100644
--- a/src/tiffvisitor_int.cpp
+++ b/src/tiffvisitor_int.cpp
@@ -1556,7 +1556,8 @@ namespace Exiv2 {
         }
         p += 2;
         uint32_t count = getULong(p, byteOrder());
-        if (count >= 0x10000000) {
+        bool     bigSonyPreview = object->tag() == 0x2001 && count*typeSize > 1024*1024;
+        if (count >= 0x10000000 || bigSonyPreview ) {
 #ifndef SUPPRESS_WARNINGS
             EXV_ERROR << "Directory " << groupName(object->group())
                       << ", entry 0x" << std::setw(4)

Is there a camera setting to manage the size of the thumbnail? Perhaps you can store smaller previews and the Digikam/Exiv2 v0.27.5 will work.

I have retired from Exiv2 and don't want to get involved in deciding how best to proceed. I don't know if digiKam use the system-wide exiv2 dynamic library or release their product with a static (or private dynamic) library that they can patch.

@clanmills
Copy link
Collaborator

While out running, I thought of three other matters to mention about this.

  1. It's a remarkable coincidence that this issue is 2001 and the Sony/Preview tag is 0x2001
  2. The way in which Sony store those previews in the Exif metadata is a violation of the Exif Specification.
  3. The change in Large allocation in tiffvisitor_int.cpp #1881 was introduced in response to "Security Researchers" creating files which violate the Sony specification (which violates the Exif spec).

So, if you want to blame somebody for this situation, please consider the security people and Sony. You may wish to thank me for promptly investigating this issue and providing patches and work-arounds.

@kevinbackhouse
Copy link
Collaborator

@clanmills: thank you for investigating this! You're always very fast to jump on these issues. I am happy to create a pull request with your proposed fix.

@kevinbackhouse kevinbackhouse self-assigned this Nov 22, 2021
@clanmills
Copy link
Collaborator

Thanks, Kevin. I do that because I enjoy the puzzle and especially the hunt.

We should port one of the fixes to 'main'. I prefer the "bigSonyPreview" fix. The threshold 1024*1024 should be a const variable that's declared once. When we set bigSonyPreview we should also test groupName(object->group()) == "Sony1"

It seems inevitable that we'll have a 0.27.6 in 2022, so a PR for 0.27-maintenance is necessary. @nehaljwani has volunteered to be the release engineer and I'm looking forward to working with him.

I'm curious to hear from @hansph and/or @cgilles about integrating the fix into digiKam. Can digiKam be patched or does it require 0.27.6 to be released? I don't know. I'd also like to hear from @hansph about setting the camera to store smaller thumbnails. That might be a quick work-around to enable him to use his camera with 0.27.5.

@hansph
Copy link
Author

hansph commented Nov 22, 2021 via email

@hansph
Copy link
Author

hansph commented Nov 22, 2021

So thanks again for the fast reply. This is what a good Open Source project distinguishes from an outstanding one.!

Does it make sense for me to compile latest git? I think it will take some weeks until a new version appears in the Manjaro stable.

I also believe that the lens may make a difference; the Zeiss is rather sharp and may produce detailed and not-so-well compressable thumbnails. So my workaround is to use a less sharp lens, lets see if this helps.

HP

@clanmills
Copy link
Collaborator

The road the code travels from my machine to yours is a mystery to me. I don't even know what Manjaro is!

I installed digikam on ubuntu and it's using Exiv2 v0.27.2.

I restarted digikam with $ env LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH digikam & and it's using my build of 0.27.5 in /usr/local/lib.

rmills@rmillsm1-ubuntu:~$ env LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH digikam &
rmills@rmillsm1-ubuntu:~$ lsof | grep exiv2
...
digikam   6249 6267 digikam             rmills  mem       REG                8,2  5227136    2113351 /usr/local/lib/libexiv2.so.0.27.5.1
...
rmills@rmillsm1-ubuntu:~$ 

When the PR for this issue is merged into branch 0.27-maintenance branch, I think you'll be good. If you're in a hurry, you can get the v0.27.5 source, apply the patch above, build and install. Happiness is possible.

Building 'main' will not help you. digikam is built to run with Exiv2 0.27 (and .1, .2, .3, .4, .5). 'main' has a new/incompatible API.

@kmilos
Copy link
Collaborator

kmilos commented Nov 23, 2021

As for the size limit, I think the Exif spec only stipulates it for the thumbnails inside JPEG APP1 (<=64 kB for the whole block), otherwise it is unlimited AFAICT for other formats? Though it does make sense to skip large ones (say 2-4MB, that seems to be the usual attachment size/upload limit for a lot of web pages), but not throw an error.

@clanmills
Copy link
Collaborator

The JPEG APP1 segment should be totally self-contained (and therefore relocatable). Sony define a preview outside the Exif data and use an absolute file offset to the preview itself. This is forbidden in the standard. The likelihood of the preview surviving a file re-write is low.

Adobe have an 'ad-hoc' method of chaining 64k segments if the Exif data is >64k. I discuss this in my book and it's supported by the tvisitor code.

When I discussed #1881 with @kevinbackhouse, I make a guess that 1mb would be sufficient. For this file, this is not sufficient. The bigSonyPreview patch above skips big previews without throwing. In the patch, I retained the 1mb limit (for simple consistency). Setting a higher threshold (2mb or 4mb) would be preferable.

@kevinbackhouse
Copy link
Collaborator

I think I would like to fix this by checking that isize is smaller than the total size of the image file. That will be more reliable than baking in an arbitrary upper limit, while also preventing a malicious image file from triggering a huge memory allocation. But it will require some extra plumbing to pass the size of the image file to this part of the code. If that sounds like a good plan, then I will start writing the code (probably not until this weekend though).

@clanmills
Copy link
Collaborator

Checking the size of the file is what is usually done. However, I couldn't find a way back through the tiff-visitor to discover the size of the stream being read!

@kevinbackhouse
Copy link
Collaborator

You're right, it is really quite awkward to get the file size to this part of the code. Also, it wouldn't even be a complete solution to the out-of-memory problem, because I think somebody could just create a file with lots of tags and quickly exhaust the memory by allocating 1MB each time.

I have attempted a different (hopefully better) solution here: #2008

@wonx
Copy link

wonx commented Dec 3, 2021

I seem to have this issue with some (but not all) pictures taken with a Sony ILCE-5000 digital camera. I noticed this problem while using the program Digikam (which internally uses exiv2 0.27.5). I get the following error:

Exiv2 exception in print action for file /home/user/DSC07110.JPG:
corrupted image metadata

However, this bug is not present in exiv2 0.27.2, and metadata is read correctly.

If it's of any help, there is a sample picture and more info linked to the bug report in Digikam's project: https://bugs.kde.org/show_bug.cgi?id=446403

@clanmills
Copy link
Collaborator

I think we've found a volunteer to work on this during "the holidays" #2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants