Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 66 additions & 21 deletions src/libOpenImageIO/exif.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ tiff_data_size(TIFFDataType tifftype)
static size_t sizes[] = { 0, 1, 1, 2, 4, 8, 1, 1, 2, 4, 8, 4, 8, 4 };
const int num_data_sizes = sizeof(sizes) / sizeof(*sizes);
int dir_index = bitcast<int>(tifftype);
if (dir_index == EXIF_UTF8_TYPE)
return 1; // Exif 3.0 UTF-8 string: one byte per element

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to handle EXIF_UTF8_TYPE in tiff_datatype_to_typedesc below as well.

    if (bitcast<int>(tifftype) == EXIF_UTF8_TYPE) {
        return TypeString;
    }

Because rawinput.cpp uses these functions together. Not a new issue though, without that the metadata will just continue to be skipped as before. Just more obvious now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; updating PR.

if (dir_index < 0 || dir_index >= num_data_sizes) {
// Inform caller about corrupted entry.
return -1;
Expand Down Expand Up @@ -216,6 +218,8 @@ tiff_datatype_to_typedesc(TIFFDataType tifftype, size_t tiffcount)
#endif
default: break;
}
if (static_cast<TIFFDataType_Exif3_Extensions>(tifftype) == EXIF_UTF8_TYPE)
return TypeString;
return TypeUnknown;
}

Expand All @@ -224,7 +228,13 @@ tiff_datatype_to_typedesc(TIFFDataType tifftype, size_t tiffcount)
cspan<uint8_t>
tiff_dir_data(const TIFFDirEntry& td, cspan<uint8_t> data)
{
size_t len = tiff_data_size(td);
size_t elemsize = tiff_data_size(TIFFDataType(td.tdir_type));
if (elemsize == 0 || elemsize == size_t(-1)) {
// Unrecognized type: we can't know how much data it has, and the
// size_t(-1) sentinel would wrap the bounds check below.
return cspan<uint8_t>();
}
size_t len = elemsize * size_t(td.tdir_count);
if (len <= 4) {
// Short data are stored in the offset field itself
return cspan<uint8_t>((const uint8_t*)&td.tdir_offset, len);
Expand Down Expand Up @@ -365,19 +375,21 @@ version4uint8_handler(const TagInfo& taginfo, const TIFFDirEntry& dir,
}


// The MakerNote is the one tag whose decoding re-enters the IFD walk, so it
// is the one that needs to know the recursion depth. TagInfo::HandlerFunc
// can't carry that, so the real work lives here and read_exif_tag calls this
// directly rather than through the handler pointer.
static void
makernote_handler(const TagInfo& /*taginfo*/, const TIFFDirEntry& dir,
cspan<uint8_t> buf, ImageSpec& spec, bool swapendian = false,
int offset_adjustment = 0)
decode_makernote(const TIFFDirEntry& dir, cspan<uint8_t> buf, ImageSpec& spec,
bool swapendian, int offset_adjustment, int depth)
{
if (tiff_data_size(dir) <= 4)
return; // sanity check

if (spec.get_string_attribute("Make") == "Canon") {
std::vector<size_t> ifdoffsets { 0 };
std::set<size_t> offsets_seen;
decode_ifd(buf, dir.tdir_offset, spec, pvt::canon_maker_tagmap_ref(),
offsets_seen, swapendian, offset_adjustment);
offsets_seen, swapendian, offset_adjustment, depth);
} else {
// Maybe we just haven't parsed the Maker metadata yet?
// Allow a second try later by just stashing the maker note offset.
Expand All @@ -386,6 +398,15 @@ makernote_handler(const TagInfo& /*taginfo*/, const TIFFDirEntry& dir,
}


static void
makernote_handler(const TagInfo& /*taginfo*/, const TIFFDirEntry& dir,
cspan<uint8_t> buf, ImageSpec& spec, bool swapendian = false,
int offset_adjustment = 0)
{
decode_makernote(dir, buf, spec, swapendian, offset_adjustment, 0);
}



static const TagInfo tiff_tag_table[] = {
// clang-format off
Expand Down Expand Up @@ -716,7 +737,10 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name,
= pvt::dataspan<uint32_t>(*dirp, buf, offset_adjustment, 2 * count);
if (dspan.empty())
return;
float* f = OIIO_ALLOCA(float, count);
// The count comes from the file: bounded by the blob length, but that
// is still far too much to put on the stack.
float* f;
OIIO_ALLOCATE_STACK_OR_HEAP(f, float, count);
for (size_t i = 0; i < count; ++i) {
// Because the values in the blob aren't 32-bit-aligned, memcpy
// them into ints to do the swapping.
Expand All @@ -742,7 +766,8 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name,
= pvt::dataspan<int32_t>(*dirp, buf, offset_adjustment, 2 * count);
if (dspan.empty())
return;
float* f = OIIO_ALLOCA(float, count);
float* f;
OIIO_ALLOCATE_STACK_OR_HEAP(f, float, count);
for (size_t i = 0; i < count; ++i) {
// Because the values in the blob aren't 32-bit-aligned, memcpy
// them into ints to do the swapping.
Expand Down Expand Up @@ -816,14 +841,25 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name,
/// integer and float data embedded in buf needs to be byte-swapped.
/// Note that *dirp has not been swapped, and so is still in the native
/// endianness of the file.
// An IFD may point at another IFD, and a hostile blob can chain them as deep
// as its own length allows -- each level costs a stack frame, so a few hundred
// KB of Exif is enough to exhaust the stack. Real files nest two or three
// deep.
static constexpr int max_ifd_depth = 32;


static void
read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan<uint8_t> buf,
bool swab, int offset_adjustment,
std::set<size_t>& ifd_offsets_seen, const TagMap& tagmap)
std::set<size_t>& ifd_offsets_seen, const TagMap& tagmap,
int depth)
{
if (depth > max_ifd_depth)
return;

if ((const uint8_t*)dirp < buf.data()
|| (const uint8_t*)dirp + sizeof(TIFFDirEntry)
>= buf.data() + buf.size()) {
> buf.data() + buf.size()) {
#if DEBUG_EXIF_READ
std::cerr << "Ignoring directory outside of the buffer.\n";
#endif
Expand Down Expand Up @@ -859,7 +895,7 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan<uint8_t> buf,
auto offset = unswapped_tdir_offset; // int stored in offset itself
if (swab)
swap_endian(&offset);
if (offset >= size_t(buf.size())) {
if (size_t(offset) + sizeof(unsigned short) > size_t(buf.size())) {
#if DEBUG_EXIF_READ
unsigned int off2 = offset;
swap_endian(&off2);
Expand Down Expand Up @@ -905,7 +941,8 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan<uint8_t> buf,
read_exif_tag(
spec, (const TIFFDirEntry*)(ifd + 2 + d * sizeof(TIFFDirEntry)),
buf, swab, offset_adjustment, ifd_offsets_seen,
dir.tdir_tag == TIFFTAG_EXIFIFD ? exif_tagmap : gps_tagmap);
dir.tdir_tag == TIFFTAG_EXIFIFD ? exif_tagmap : gps_tagmap,
depth + 1);
#if DEBUG_EXIF_READ
std::cerr << "> End EXIF\n";
#endif
Expand All @@ -915,7 +952,7 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan<uint8_t> buf,
auto offset = unswapped_tdir_offset; // int stored in offset itself
if (swab)
swap_endian(&offset);
if (offset >= size_t(buf.size())) {
if (size_t(offset) + sizeof(unsigned short) > size_t(buf.size())) {
#if DEBUG_EXIF_READ
unsigned int off2 = offset;
swap_endian(&off2);
Expand All @@ -933,7 +970,8 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan<uint8_t> buf,
std::cerr << "Now we've seen offset " << offset << "\n";
#endif
const unsigned char* ifd = ((const unsigned char*)buf.data() + offset);
unsigned short ndirs = *(const unsigned short*)ifd;
unsigned short ndirs;
memcpy(&ndirs, ifd, sizeof(ndirs)); // hoop jumping for ubsan
if (swab)
swap_endian(&ndirs);
#if DEBUG_EXIF_READ
Expand All @@ -943,17 +981,22 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan<uint8_t> buf,
<< "\n";
#endif
for (int d = 0; d < ndirs; ++d)
read_exif_tag(
spec, (const TIFFDirEntry*)(ifd + 2 + d * sizeof(TIFFDirEntry)),
buf, swab, offset_adjustment, ifd_offsets_seen, exif_tagmap);
read_exif_tag(spec,
(const TIFFDirEntry*)(ifd + 2
+ d * sizeof(TIFFDirEntry)),
buf, swab, offset_adjustment, ifd_offsets_seen,
exif_tagmap, depth + 1);
#if DEBUG_EXIF_READ
std::cerr << "> End Interoperability\n\n";
#endif
} else {
// Everything else -- use our table to handle the general case
const TagInfo* taginfo = tagmap.find(dir.tdir_tag);
if (taginfo && !spec.extra_attribs.contains(taginfo->name)) {
if (taginfo->handler)
if (taginfo->handler == makernote_handler)
decode_makernote(dir, buf, spec, swab, offset_adjustment,
depth + 1);
else if (taginfo->handler)
taginfo->handler(*taginfo, dir, buf, spec, swab,
offset_adjustment);
else if (taginfo->tifftype != TIFF_NOTYPE)
Expand Down Expand Up @@ -1094,7 +1137,7 @@ encode_exif_entry(const ParamValue& p, int tag, std::vector<TIFFDirEntry>& dirs,
bool
pvt::decode_ifd(cspan<uint8_t> buf, size_t ifd_offset, ImageSpec& spec,
const TagMap& tag_map, std::set<size_t>& ifd_offsets_seen,
bool swab, int offset_adjustment)
bool swab, int offset_adjustment, int depth)
{
// Read the directory that the header pointed to. It should contain
// some number of directory entries containing tags to process.
Expand All @@ -1112,7 +1155,8 @@ pvt::decode_ifd(cspan<uint8_t> buf, size_t ifd_offset, ImageSpec& spec,
for (int d = 0; d < ndirs; ++d)
read_exif_tag(spec,
(const TIFFDirEntry*)(ifd + 2 + d * sizeof(TIFFDirEntry)),
buf, swab, offset_adjustment, ifd_offsets_seen, tag_map);
buf, swab, offset_adjustment, ifd_offsets_seen, tag_map,
depth);
return true;
}

Expand Down Expand Up @@ -1235,7 +1279,8 @@ decode_exif(cspan<uint8_t> exif, ImageSpec& spec)
// itself is also helpful in this area.
if (exif.size() < sizeof(TIFFHeader))
return false;
TIFFHeader head = *(const TIFFHeader*)exif.data();
TIFFHeader head;
memcpy(&head, exif.data(), sizeof(head)); // may be unaligned
if (head.tiff_magic != 0x4949 && head.tiff_magic != 0x4d4d)
return false;
bool host_little = littleendian();
Expand Down
35 changes: 28 additions & 7 deletions src/libOpenImageIO/exif.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,32 @@ namespace pvt {



// Byte length of a directory entry's data, or 0 if we can't know it.
// tiff_data_size() reports an unrecognized type as size_t(-1), which must
// never reach the bounds arithmetic below: multiplied by the count it wraps,
// and a wrapped length passes any `offset + len > size` test.
inline size_t
dirdata_size(const TIFFDirEntry& td)
{
size_t elemsize = tiff_data_size(TIFFDataType(td.tdir_type));
if (elemsize == 0 || elemsize == size_t(-1))
return 0;
return elemsize * size_t(td.tdir_count);
}



inline const void*
dataptr(const TIFFDirEntry& td, cspan<uint8_t> data, int offset_adjustment)
{
size_t len = tiff_data_size(td);
size_t len = dirdata_size(td);
if (len == 0)
return nullptr; // unknown type or no data
if (len <= 4)
return (const char*)&td.tdir_offset;
else {
int offset = td.tdir_offset + offset_adjustment;
if (offset < 0 || size_t(offset) + len > std::size(data))
int64_t offset = int64_t(td.tdir_offset) + offset_adjustment;
if (offset < 0 || uint64_t(offset) + len > uint64_t(std::size(data)))
return nullptr; // out of bounds!
return (const char*)data.data() + offset;
}
Expand All @@ -57,13 +74,15 @@ inline cspan<uint8_t>
dataspan(const TIFFDirEntry& td, cspan<uint8_t> data, int offset_adjustment,
size_t count)
{
size_t len = tiff_data_size(td);
size_t len = dirdata_size(td);
OIIO_DASSERT(len == sizeof(T) * count);
if (len == 0)
return {}; // unknown type or no data
if (len <= 4)
return { (const uint8_t*)&td.tdir_offset, span_size_t(len) };
else {
int offset = td.tdir_offset + offset_adjustment;
if (offset < 0 || size_t(offset) + len > std::size(data))
int64_t offset = int64_t(td.tdir_offset) + offset_adjustment;
if (offset < 0 || uint64_t(offset) + len > uint64_t(std::size(data)))
return {}; // out of bounds! return empty span
return { data.data() + offset, span_size_t(len) };
}
Expand Down Expand Up @@ -140,10 +159,12 @@ void append_tiff_dir_entry (std::vector<TIFFDirEntry> &dirs,
size_t offset_override = 0,
OIIO::endian endianreq = OIIO::endian::native);

// Decode one IFD and everything it points at. `depth` is the IFD nesting
// level, incremented on each recursive step and used to bound the recursion.
bool decode_ifd (cspan<uint8_t> buf, size_t ifd_offset,
ImageSpec &spec, const TagMap& tag_map,
std::set<size_t>& ifd_offsets_seen, bool swab=false,
int offset_adjustment=0);
int offset_adjustment=0, int depth=0);

void encode_canon_makernote (std::vector<char>& exifblob,
std::vector<TIFFDirEntry> &exifdirs,
Expand Down
17 changes: 17 additions & 0 deletions testsuite/jpeg-corrupt/ref/out-alt2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg
YResolution: 300
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-recursive-ifd.jpg
src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg
SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D
channel list: R, G, B
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-utf8-type.jpg
src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
ImageDescription: ""
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-deep-ifds.jpg
src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
oiio:ColorSpace: "srgb_rec709_scene"
corrupt-icc-4551.jpg
DCT coefficient (lossy) or spatial difference (lossless) out of range
Reading src/corrupt-icc-4552.jpg
Expand Down
17 changes: 17 additions & 0 deletions testsuite/jpeg-corrupt/ref/out-alt3.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg
YResolution: 300
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-recursive-ifd.jpg
src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg
SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D
channel list: R, G, B
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-utf8-type.jpg
src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
ImageDescription: ""
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-deep-ifds.jpg
src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
oiio:ColorSpace: "srgb_rec709_scene"
corrupt-icc-4551.jpg
Reading src/corrupt-icc-4552.jpg
src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
Expand Down
17 changes: 17 additions & 0 deletions testsuite/jpeg-corrupt/ref/out-alt4.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg
YResolution: 300
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-recursive-ifd.jpg
src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg
SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D
channel list: R, G, B
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-utf8-type.jpg
src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
ImageDescription: ""
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-deep-ifds.jpg
src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
oiio:ColorSpace: "srgb_rec709_scene"
corrupt-icc-4551.jpg
DCT coefficient out of range
Reading src/corrupt-icc-4552.jpg
Expand Down
17 changes: 17 additions & 0 deletions testsuite/jpeg-corrupt/ref/out-alt5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg
YResolution: 300
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-recursive-ifd.jpg
src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg
SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D
channel list: R, G, B
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-utf8-type.jpg
src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
ImageDescription: ""
oiio:ColorSpace: "srgb_rec709_scene"
Reading src/corrupt-exif-deep-ifds.jpg
src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg
SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
channel list: Y
oiio:ColorSpace: "srgb_rec709_scene"
corrupt-icc-4551.jpg
iconvert ERROR copying "src/corrupt-icc-4551.jpg" to "out-4551.jpg" :
JPEG error: Corrupt JPEG data: bad Huffman code ("src/corrupt-icc-4551.jpg")
Expand Down
Loading
Loading