Project

General

Profile

Bug #11059

Crash with N900 DNG files

Added by Sven Gaerner over 3 years ago. Updated over 3 years ago.

Status:
Fixed
Priority:
Low
Assignee:
-
Category:
Lighttable
Target version:
Start date:
06/20/2016
Due date:
% Done:

100%

Estimated time:
Affected Version:
2.0.0
System:
FreeBSD
bitness:
64-bit
hardware architecture:
amd64/x86

Description

When in lighttable mode and viewing DNG files, DT crashes with an invalid memory access in src/external/rawspeed/RawSpeed/TiffIFD.cpp (TiffIFD::TiffIFD(FileMap* f, uint32 offset)).

Using the std::string for string comparism of the string 'Adobe' causes the crash because the buffer might not be 0-terminated.

Attached is a patch that fixes this issue for me on OpenBSD 5.9 64bit.

fix-invalid-memory-access.patch (1.53 KB) fix-invalid-memory-access.patch Sven Gaerner, 06/20/2016 12:26 AM

Associated revisions

Revision abbc577b (diff)
Added by Roman Lebedev over 3 years ago

rawspeed: TiffIFD::parseDngPrivateData(): compare with "Adobe\0" via memcmp()

1. The tag can be non-zero-terminated
(just replace \0 with anything else with hex editor)
2. We know the exact length and data that
tag should contain ("Adobe\0", 6)
=> memcmp is the only sane choice here.

There are probably a lot other cases like this.

Fixes #11059

Revision cb541466 (diff)
Added by Roman Lebedev over 3 years ago

rawspeed: TiffIFD::parseDngPrivateData(): compare with "Adobe\0" via memcmp()

1. The tag can be non-zero-terminated
(just replace \0 with anything else with hex editor)
2. We know the exact length and data that
tag should contain ("Adobe\0", 6)
=> memcmp is the only sane choice here.

There are probably a lot other cases like this.

Fixes #11059

(cherry picked from commit abbc577bf87c211e73f663a78173168faaf8aca3)

History

#1 Updated by Tobias Ellinghaus over 3 years ago

Could you please provide a sample DNG for testing this? Thank you.

#2 Updated by Sven Gaerner over 3 years ago

If you look into the DNG with an editor there is no "Adobe" string available. I don't know if the DNG is standard compliant, but with this patch it can be read and displayed, but without any whitebalance information available.

Thanks for looking into this issue.

#3 Updated by Roman Lebedev over 3 years ago

Sven Gaerner wrote:

If you look into the DNG with an editor there is no "Adobe" string available. I don't know if the DNG is standard compliant, but with this patch it can be read and displayed, but without any whitebalance information available.

Thanks for looking into this issue.

This did not result in dng actually being attached...

#4 Updated by Sven Gaerner over 3 years ago

I did another test on OpenBSD and Linux (Debian 8) with an empty directory for configuration and test. The test was:

  • start darktable
  • import folder containing just the uploaded DNG, no XMP file
  • see preview in lightable view
  • press D to switch to darkroom view
  • observe crash

On Linux I compiled darktable 2.0.4 from sources using the default options (just run cmake ~/src/darktable) and did not observe this crash.

On OpenBSD I did the nearly the same (I had to pass some extra options to make cmake select the required compiler). I also tested version 2.0.0 to 2.0.4, but in all cases I observed the crash. The cmake call was

env CPPFLAGS+=-I/usr/local/include LDFLAGS+=-L/usr/local/lib CFLAGS+=-msse3 CXXFLAGS+=-msse3        \
    cmake -G Ninja -DCMAKE_INSTALL_PREFIX=/home/sven/local -DCMAKE_BUILD_TYPE=Release               \
          -DCMAKE_C_COMPILER=/usr/local/bin/egcc -DCMAKE_CXX_COMPILER=/usr/local/bin/eg++           \
          -DOPENJPEG_INLCUDE_DIR=/usr/local/include/openjpeg-1.5 -DCUSTOM_FLAGS=1                   \
          -DBINARY_PACKAGE_BUILD=1 ~/src/darktable

I also created a package for the same version using the OpenBSD ports system. These versions also core dumped.

I must correct my assumption. The crash occurs when the std::string is removed from stack and its destructor is run. This crashes also if I change the code to:

string id((const char*)data, size);

or
string id;
id.assign((const char*)data, size);

The problem occurs only if RawSpeed is compiled with -O2, compiling with -O0 or -O1 won't trigger the crash.

#5 Updated by Sven Gaerner over 3 years ago

I tracked it down to a std::string problem.

  • create std::string id
  • call compare method
  • no Private data, call ThrowTPE()
  • unwind stack and run id destructor

This last call is causing the crash, so I'm not sure if this is a darktable problem.

So the fix, using strcmp instead of the std::string is just a workaround that hides the problem.

Sorry for the noise.

#6 Updated by Roman Lebedev over 3 years ago

Looking from the perspective at this:
1. The tag can be non-zero-terminated (just replace \0 with anything else with hex editor)
2. BUT you know the exact length and data that tag should contain ("Adobe\0", 6)

=> memcmp is the only sane choice here.

#7 Updated by Roman Lebedev over 3 years ago

  • % Done changed from 0 to 100
  • Status changed from New to Fixed

#8 Updated by Roman Lebedev over 3 years ago

  • Target version set to 2.2.0

Also available in: Atom PDF

Go to top