From cc827d59a871dba32bcf9b29469ff091e6b79293 Mon Sep 17 00:00:00 2001 From: Andreas Huggel Date: Thu, 13 May 2004 16:14:16 +0000 Subject: [PATCH] Added len argument and boundary checks to various read functions --- src/ifd.cpp | 123 +++++++++++++++++++++++++++++----------------- src/ifd.hpp | 21 ++++++-- src/makernote.cpp | 7 +-- 3 files changed, 97 insertions(+), 54 deletions(-) diff --git a/src/ifd.cpp b/src/ifd.cpp index 4deb71d5..705486e1 100644 --- a/src/ifd.cpp +++ b/src/ifd.cpp @@ -20,14 +20,14 @@ */ /* File: ifd.cpp - Version: $Name: $ $Revision: 1.17 $ + Version: $Name: $ $Revision: 1.18 $ 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.17 $ $RCSfile: ifd.cpp,v $") +EXIV2_RCSID("@(#) $Name: $ $Revision: 1.18 $ $RCSfile: ifd.cpp,v $") // ***************************************************************************** // included header files @@ -197,40 +197,54 @@ namespace Exiv2 { } } - int Ifd::read(const char* buf, ByteOrder byteOrder, long offset) + int Ifd::read(const char* buf, long len, ByteOrder byteOrder, long offset) { - offset_ = offset; - + int rc = 0; + long o = 0; Ifd::PreEntries preEntries; - int n = getUShort(buf, byteOrder); - long o = 2; + if (len < 2) rc = 6; + if (rc == 0) { + offset_ = offset; + int n = getUShort(buf, byteOrder); + o = 2; - for (int i = 0; i < n; ++i) { - Ifd::PreEntry pe; - pe.tag_ = getUShort(buf+o, byteOrder); - pe.type_ = getUShort(buf+o+2, byteOrder); - pe.count_ = getULong(buf+o+4, byteOrder); - pe.size_ = pe.count_ * TypeInfo::typeSize(TypeId(pe.type_)); - pe.offsetLoc_ = o + 8; - pe.offset_ = pe.size_ > 4 ? getULong(buf+o+8, byteOrder) : 0; - preEntries.push_back(pe); - o += 12; - } - if (alloc_) { - memcpy(pNext_, buf + o, 4); + for (int i = 0; i < n; ++i) { + if (len < o + 12) { + rc = 6; + break; + } + Ifd::PreEntry pe; + pe.tag_ = getUShort(buf + o, byteOrder); + pe.type_ = getUShort(buf + o + 2, byteOrder); + pe.count_ = getULong(buf + o + 4, byteOrder); + pe.size_ = pe.count_ * TypeInfo::typeSize(TypeId(pe.type_)); + pe.offsetLoc_ = o + 8; + pe.offset_ = pe.size_ > 4 ? getULong(buf + o + 8, byteOrder) : 0; + preEntries.push_back(pe); + o += 12; + } } - else { - pNext_ = const_cast(buf + o); + if (rc == 0) { + if (len < o + 4) { + rc = 6; + } + else { + if (alloc_) { + memcpy(pNext_, buf + o, 4); + } + else { + pNext_ = const_cast(buf + o); + } + next_ = getULong(buf + o, byteOrder); + } } - next_ = getULong(buf+o, byteOrder); - // Set the offset of the first data entry outside of the IFD. // At the same time we guess the offset of the IFD, if it was not // given. The guess is based on the assumption that the smallest offset // points to a data buffer directly following the IFD. Subsequently all // offsets of IFD entries will need to be recalculated. - if (preEntries.size() > 0) { + if (rc == 0 && preEntries.size() > 0) { // Find the entry with the smallest offset Ifd::PreEntries::const_iterator i = std::min_element( preEntries.begin(), preEntries.end(), cmpPreEntriesByOffset); @@ -242,31 +256,42 @@ namespace Exiv2 { offset_ = i->offset_ - size(); } // Set the offset of the first data entry outside of the IFD - dataOffset_ = i->offset_; + if (static_cast(len) < i->offset_ - offset_) { + rc = 6; + } + else { + dataOffset_ = i->offset_; + } } } - // Convert the pre-IFD entries to the actual entries, assign the data // to each IFD entry and calculate relative offsets, relative to the // start of the IFD - entries_.clear(); - int idx = 0; - const Ifd::PreEntries::iterator begin = preEntries.begin(); - const Ifd::PreEntries::iterator end = preEntries.end(); - for (Ifd::PreEntries::iterator i = begin; i != end; ++i) { - Entry e(alloc_); - 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_); - // 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_)); - this->add(e); + if (rc == 0) { + entries_.clear(); + int idx = 0; + const Ifd::PreEntries::iterator begin = preEntries.begin(); + const Ifd::PreEntries::iterator end = preEntries.end(); + for (Ifd::PreEntries::iterator i = begin; i != end; ++i) { + Entry e(alloc_); + 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_) { + rc = 6; + break; + } + // 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_)); + this->add(e); + } } + if (rc) this->clear(); - return 0; + return rc; } // Ifd::read Ifd::const_iterator Ifd::findIdx(int idx) const @@ -299,14 +324,19 @@ namespace Exiv2 { } int Ifd::readSubIfd( - Ifd& dest, const char* buf, ByteOrder byteOrder, uint16 tag + Ifd& dest, const char* buf, long len, ByteOrder byteOrder, uint16 tag ) const { int rc = 0; const_iterator pos = findTag(tag); if (pos != entries_.end()) { - uint32 offset = getULong(pos->data(), byteOrder); - rc = dest.read(buf + offset, byteOrder, offset); + long offset = getULong(pos->data(), byteOrder); + if (len < offset) { + rc = 6; + } + else { + rc = dest.read(buf + offset, len - offset, byteOrder, offset); + } } return rc; } // Ifd::readSubIfd @@ -371,6 +401,7 @@ namespace Exiv2 { else { pNext_ = 0; } + next_ = 0; offset_ = 0; dataOffset_ = 0; } // Ifd::clear diff --git a/src/ifd.hpp b/src/ifd.hpp index 81ac099b..9ea5d181 100644 --- a/src/ifd.hpp +++ b/src/ifd.hpp @@ -21,7 +21,7 @@ /*! @file ifd.hpp @brief Encoding and decoding of IFD (Image File Directory) data - @version $Name: $ $Revision: 1.16 $ + @version $Name: $ $Revision: 1.17 $ @author Andreas Huggel (ahu) ahuggel@gmx.net @date 09-Jan-04, ahu: created @@ -310,6 +310,7 @@ namespace Exiv2 { @param buf Pointer to the data to decode. The buffer must start with the IFD data (unlike the readSubIfd() method). + @param len Number of bytes in the data buffer @param byteOrder Applicable byte order (little or big endian). @param offset (Optional) offset of the IFD from the start of the TIFF header, if known. If not given, the offset will be guessed @@ -317,9 +318,12 @@ namespace Exiv2 { directory entries points to a data buffer immediately follwing the IFD. - @return 0 if successful + @return 0 if successful;
+ 6 if the data buffer is too small, e.g., if an offset points + beyond the provided buffer. The IFD is cleared in this + case. */ - int read(const char* buf, ByteOrder byteOrder, long offset =0); + int read(const char* buf, long len, ByteOrder byteOrder, long offset =0); /*! @brief Read a sub-IFD from the location pointed to by the directory entry with the given tag. @@ -327,13 +331,20 @@ namespace Exiv2 { @param dest References the destination IFD. @param buf The data buffer to read from. The buffer must contain all Exif data starting from the TIFF header (unlike the read() method). + @param len Number of bytes in the data buffer @param byteOrder Applicable byte order (little or big endian). @param tag Tag to look for. - @return 0 if successful + @return 0 if successful;
+ 6 if reading the sub-IFD failed (see read() above) or + the location pointed to by the directory entry with the + given tag is outside of the data buffer. + + @note It is not considered an error if the tag cannot be found in the + IFD. 0 is returned and no action is taken in this case. */ int readSubIfd( - Ifd& dest, const char* buf, ByteOrder byteOrder, uint16 tag + Ifd& dest, const char* buf, long len, ByteOrder byteOrder, uint16 tag ) const; /*! @brief Copy the IFD to a data array, update the offsets of the IFD and diff --git a/src/makernote.cpp b/src/makernote.cpp index c8525fe2..acadd35b 100644 --- a/src/makernote.cpp +++ b/src/makernote.cpp @@ -20,13 +20,13 @@ */ /* File: makernote.cpp - Version: $Name: $ $Revision: 1.17 $ + Version: $Name: $ $Revision: 1.18 $ Author(s): Andreas Huggel (ahu) History: 18-Feb-04, ahu: created */ // ***************************************************************************** #include "rcsid.hpp" -EXIV2_RCSID("@(#) $Name: $ $Revision: 1.17 $ $RCSfile: makernote.cpp,v $") +EXIV2_RCSID("@(#) $Name: $ $Revision: 1.18 $ $RCSfile: makernote.cpp,v $") // Define DEBUG_MAKERNOTE to output debug information to std::cerr #undef DEBUG_MAKERNOTE @@ -162,7 +162,7 @@ namespace Exiv2 { int rc = 0; if (!prefix_.empty()) { // Check if makernote is long enough and starts with prefix - if ( len <= static_cast(prefix_.size()) + if ( len < static_cast(prefix_.size()) || prefix_ != std::string(buf, prefix_.size())) rc = 2; } if (!absOffset_) { @@ -171,6 +171,7 @@ namespace Exiv2 { } if (rc == 0) { rc = ifd_.read(buf + prefix_.size(), + len - prefix_.size(), byteOrder_, offset + prefix_.size()); }