Add pbox_end param to BmffImage::boxHandler to enforce box nesting.

Exiv2 · Jul 23, 2021 · 8c64e9a · 8c64e9a
1 parent adb586a
commit 8c64e9a
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 12 deletions.
diff --git a/include/exiv2/bmffimage.hpp b/include/exiv2/bmffimage.hpp
@@ -128,10 +128,13 @@ namespace Exiv2
         /*!
           @brief recursiveBoxHandler
           @throw Error if we visit a box more than once
+          @param pbox_end The end location of the parent box. Boxes are
+              nested, so we must not read beyond this.
           @return address of next box
           @warning This function should only be called by readMetadata()
          */
-        long boxHandler(std::ostream& out=std::cout, Exiv2::PrintStructureOption option=kpsNone,int depth = 0);
+        long boxHandler(std::ostream& out, Exiv2::PrintStructureOption option,
+                        const long pbox_end, int depth);
         std::string indent(int i)
         {
             return std::string(2*i,' ');

diff --git a/src/bmffimage.cpp b/src/bmffimage.cpp
@@ -183,9 +183,12 @@ namespace Exiv2
         return result;
     }
 
-    long BmffImage::boxHandler(std::ostream& out /* = std::cout*/ , Exiv2::PrintStructureOption option /* = kpsNone */,int depth /* =0 */)
+    long BmffImage::boxHandler(std::ostream& out /* = std::cout*/ ,
+                               Exiv2::PrintStructureOption option /* = kpsNone */,
+                               const long pbox_end,
+                               int depth)
     {
-        long result = static_cast<long>(io_->size());
+        long result = pbox_end;
         long address = io_->tell();
         // never visit a box twice!
         if ( depth == 0 ) visits_.clear();
@@ -200,6 +203,8 @@ namespace Exiv2
 #endif
 
         BmffBoxHeader box = {0, 0};
+        size_t hdrsize = sizeof(box);
+        enforce(hdrsize <= static_cast<size_t>(pbox_end - address), Exiv2::kerCorruptedMetadata);
         if (io_->read(reinterpret_cast<byte*>(&box), sizeof(box)) != sizeof(box))
             return result;
 
@@ -214,6 +219,9 @@ namespace Exiv2
         }
 
         if (box.length == 1) {
+            // The box size is encoded as a uint64_t, so we need to read another 8 bytes.
+            hdrsize += 8;
+            enforce(hdrsize <= static_cast<size_t>(pbox_end - address), Exiv2::kerCorruptedMetadata);
             DataBuf data(8);
             io_->read(data.pData_, data.size_);
             const uint64_t sz = getULongLong(data.pData_, endian_);
@@ -232,8 +240,10 @@ namespace Exiv2
 
         // read data in box and restore file position
         long restore = io_->tell();
-        enforce(box.length >= 8, Exiv2::kerCorruptedMetadata);
-        DataBuf data(box.length - 8);
+        enforce(box.length >= hdrsize, Exiv2::kerCorruptedMetadata);
+        enforce(box.length - hdrsize <= static_cast<size_t>(pbox_end - restore), Exiv2::kerCorruptedMetadata);
+        DataBuf data(box.length - hdrsize);
+        const long box_end = restore + data.size_;
         io_->read(data.pData_, data.size_);
         io_->seek(restore, BasicIo::beg);
 
@@ -271,7 +281,7 @@ namespace Exiv2
 
                 io_->seek(skip, BasicIo::cur);
                 while (n-- > 0) {
-                    io_->seek(boxHandler(out,option,depth + 1), BasicIo::beg);
+                    io_->seek(boxHandler(out,option,box_end,depth + 1), BasicIo::beg);
                 }
             } break;
 
@@ -311,7 +321,7 @@ namespace Exiv2
                 }
                 io_->seek(skip, BasicIo::cur);
                 while (io_->tell() < static_cast<long>((address + box.length))) {
-                    io_->seek(boxHandler(out,option,depth + 1), BasicIo::beg);
+                    io_->seek(boxHandler(out,option,box_end,depth + 1), BasicIo::beg);
                 }
                 // post-process meta box to recover Exif and XMP
                 if (box.type == TAG_meta) {
@@ -436,7 +446,7 @@ namespace Exiv2
                 }
                 if (name == "cano") {
                     while (io_->tell() < static_cast<long>(address + box.length)) {
-                        io_->seek(boxHandler(out,option,depth + 1), BasicIo::beg);
+                        io_->seek(boxHandler(out,option,box_end,depth + 1), BasicIo::beg);
                     }
                 } else if ( name == "xmp" ) {
                     parseXmp(box.length,io_->tell());
@@ -567,9 +577,10 @@ namespace Exiv2
         xmpID_     = unknownID_;
 
         long address = 0;
-        while (address < static_cast<long>(io_->size())) {
+        const long file_end = static_cast<long>(io_->size());
+        while (address < file_end) {
             io_->seek(address, BasicIo::beg);
-            address = boxHandler(std::cout,kpsNone);
+            address = boxHandler(std::cout,kpsNone,file_end,0);
         }
         bReadMetadata_ = true;
     }  // BmffImage::readMetadata
@@ -600,9 +611,10 @@ namespace Exiv2
                 IoCloser closer(*io_);
 
                 long   address = 0;
-                while (address < static_cast<long>(io_->size())) {
+                const long file_end = static_cast<long>(io_->size());
+                while (address < file_end) {
                     io_->seek(address, BasicIo::beg);
-                    address = boxHandler(out,option,depth);
+                    address = boxHandler(out,option,file_end,depth);
                 }
             }; break;
         }