From afe338162b80ecbcdf55f26b254fdde3229b584d Mon Sep 17 00:00:00 2001 From: Andreas Huggel Date: Mon, 31 May 2004 16:45:23 +0000 Subject: [PATCH] Improved handling of corrupt IFDs: Truncate field if offset points outside of the buffer --- src/ifd.cpp | 79 ++++++++++++++++++++++++++--------------------------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/src/ifd.cpp b/src/ifd.cpp index d1f7b527..e331f630 100644 --- a/src/ifd.cpp +++ b/src/ifd.cpp @@ -20,14 +20,14 @@ */ /* File: ifd.cpp - Version: $Name: $ $Revision: 1.19 $ + Version: $Name: $ $Revision: 1.20 $ Author(s): Andreas Huggel (ahu) History: 26-Jan-04, ahu: created 11-Feb-04, ahu: isolated as a component */ // ***************************************************************************** #include "rcsid.hpp" -EXIV2_RCSID("@(#) $Name: $ $Revision: 1.19 $ $RCSfile: ifd.cpp,v $") +EXIV2_RCSID("@(#) $Name: $ $Revision: 1.20 $ $RCSfile: ifd.cpp,v $") // ***************************************************************************** // included header files @@ -199,10 +199,6 @@ namespace Exiv2 { int Ifd::read(const char* buf, long len, ByteOrder byteOrder, long offset) { -// Todo: remove debug output -std::cerr << "Entering Ifd::read for " << ExifTags::ifdName(ifdId_) - << ", len = " << len << "\n"; - int rc = 0; long o = 0; Ifd::PreEntries preEntries; @@ -215,11 +211,10 @@ std::cerr << "Entering Ifd::read for " << ExifTags::ifdName(ifdId_) for (int i = 0; i < n; ++i) { if (len < o + 12) { - -// Todo: remove debug output -std::cerr << "Buffer is not large enough for " - << ExifTags::ifdName(ifdId_) << " entry " << i << "\n"; - + // Todo: How to handle debug output like this + std::cerr << "Error: " << ExifTags::ifdName(ifdId_) + << " entry " << i + 1 + << " lies outside of the IFD memory buffer.\n"; rc = 6; break; } @@ -236,10 +231,10 @@ std::cerr << "Buffer is not large enough for " } if (rc == 0) { if (len < o + 4) { - -// Todo: remove debug output -std::cerr << "Buffer is not large enough for pointer to the next IFD\n"; - + // Todo: How to handle debug output like this + std::cerr << "Error: " << ExifTags::ifdName(ifdId_) + << " memory of the pointer to the next IFD" + << " lies outside of the IFD memory buffer.\n"; rc = 6; } else { @@ -270,17 +265,15 @@ std::cerr << "Buffer is not large enough for pointer to the next IFD\n"; } // Set the offset of the first data entry outside of the IFD if (static_cast(len) < i->offset_ - offset_) { - -// Todo: remove debug output -std::cerr << "Offset of the 1st data entry of " << ExifTags::ifdName(ifdId_) - << " is out of bounds:\n" - << "Offset = " << i->offset_ - offset_ - << ", exceeds buffer size by " - << i->offset_ - offset_ - static_cast(len) - << " Bytes\n"; - -// Todo: Can I truncate the data or set size to 0? What are the side effects? - + // Todo: How to handle debug output like this + std::cerr << "Error: Offset of the 1st data entry of " + << ExifTags::ifdName(ifdId_) + << " is out of bounds:\n" + << " Offset = " << i->offset_ - offset_ + << ", exceeds buffer size by " + << i->offset_ - offset_ + - static_cast(len) + << " Bytes\n"; rc = 6; } else { @@ -301,21 +294,27 @@ std::cerr << "Offset of the 1st data entry of " << ExifTags::ifdName(ifdId_) e.setIfdId(ifdId_); e.setIdx(++idx); e.setTag(i->tag_); - // Set the offset to the data, relative to start of IFD - e.setOffset(i->size_ > 4 ? i->offset_ - offset_ : i->offsetLoc_); - if (static_cast(len) < e.offset() + i->size_) { - -// Todo: remove debug output -std::cerr << "Upper boundary of data for " << ExifTags::ifdName(ifdId_) - << " entry " << i - begin - << " is out of bounds:\nOffset = " << e.offset() - << ", size = " << i->size_ << ", exceeds buffer size by " - << e.offset() + i->size_ - static_cast(len) - << " Bytes\n"; - - rc = 6; - break; + uint32 tmpOffset = + i->size_ > 4 ? i->offset_ - offset_ : i->offsetLoc_; + if (static_cast(len) < tmpOffset + i->size_) { + // Todo: How to handle debug output like this + std::cerr << "Warning: Upper boundary of data for " + << ExifTags::ifdName(ifdId_) + << " entry " << i - begin + 1 + << " is out of bounds:\n" + << " Offset = " << tmpOffset + << ", size = " << i->size_ + << ", exceeds buffer size by " + << tmpOffset + i->size_ + - static_cast(len) + << " Bytes; Truncating the data.\n"; + // Truncate the entry + i->size_ = 0; + i->count_ = 0; + tmpOffset = i->offsetLoc_; } + // Set the offset to the data, relative to start of IFD + e.setOffset(tmpOffset); // Set the size to at least for bytes to accomodate offset-data e.setValue(i->type_, i->count_, buf + e.offset(), std::max(long(4), i->size_));