1From 783b3a6ff15ed6f82a8f8e6c8a6f3b84a9b04d4b Mon Sep 17 00:00:00 2001 2From: Kevin Backhouse <kevinbackhouse@github.com> 3Date: Mon, 19 Apr 2021 18:06:00 +0100 4Subject: [PATCH] Improve bound checking in WebPImage::doWriteMetadata() 5 6--- 7 src/webpimage.cpp | 41 ++++++++++++++++++++++++++++++----------- 8 1 file changed, 30 insertions(+), 11 deletions(-) 9 10diff --git a/src/webpimage.cpp b/src/webpimage.cpp 11index 4ddec544c..fee110bca 100644 12--- a/src/webpimage.cpp 13+++ b/src/webpimage.cpp 14@@ -145,7 +145,7 @@ namespace Exiv2 { 15 DataBuf chunkId(WEBP_TAG_SIZE+1); 16 chunkId.pData_ [WEBP_TAG_SIZE] = '\0'; 17 18- io_->read(data, WEBP_TAG_SIZE * 3); 19+ readOrThrow(*io_, data, WEBP_TAG_SIZE * 3, Exiv2::kerCorruptedMetadata); 20 uint64_t filesize = Exiv2::getULong(data + WEBP_TAG_SIZE, littleEndian); 21 22 /* Set up header */ 23@@ -185,13 +185,20 @@ namespace Exiv2 { 24 case we have any exif or xmp data, also check 25 for any chunks with alpha frame/layer set */ 26 while ( !io_->eof() && (uint64_t) io_->tell() < filesize) { 27- io_->read(chunkId.pData_, WEBP_TAG_SIZE); 28- io_->read(size_buff, WEBP_TAG_SIZE); 29- long size = Exiv2::getULong(size_buff, littleEndian); 30+ readOrThrow(*io_, chunkId.pData_, WEBP_TAG_SIZE, Exiv2::kerCorruptedMetadata); 31+ readOrThrow(*io_, size_buff, WEBP_TAG_SIZE, Exiv2::kerCorruptedMetadata); 32+ const uint32_t size_u32 = Exiv2::getULong(size_buff, littleEndian); 33+ 34+ // Check that `size_u32` is safe to cast to `long`. 35+ enforce(size_u32 <= static_cast<size_t>(std::numeric_limits<unsigned int>::max()), 36+ Exiv2::kerCorruptedMetadata); 37+ const long size = static_cast<long>(size_u32); 38 DataBuf payload(size); 39- io_->read(payload.pData_, payload.size_); 40- byte c; 41- if ( payload.size_ % 2 ) io_->read(&c,1); 42+ readOrThrow(*io_, payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); 43+ if ( payload.size_ % 2 ) { 44+ byte c; 45+ readOrThrow(*io_, &c, 1, Exiv2::kerCorruptedMetadata); 46+ } 47 48 /* Chunk with information about features 49 used in the file. */ 50@@ -199,6 +206,7 @@ namespace Exiv2 { 51 has_vp8x = true; 52 } 53 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8X) && !has_size) { 54+ enforce(size >= 10, Exiv2::kerCorruptedMetadata); 55 has_size = true; 56 byte size_buf[WEBP_TAG_SIZE]; 57 58@@ -227,6 +235,7 @@ namespace Exiv2 { 59 } 60 #endif 61 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8) && !has_size) { 62+ enforce(size >= 10, Exiv2::kerCorruptedMetadata); 63 has_size = true; 64 byte size_buf[2]; 65 66@@ -244,11 +253,13 @@ namespace Exiv2 { 67 68 /* Chunk with with lossless image data. */ 69 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8L) && !has_alpha) { 70+ enforce(size >= 5, Exiv2::kerCorruptedMetadata); 71 if ((payload.pData_[4] & WEBP_VP8X_ALPHA_BIT) == WEBP_VP8X_ALPHA_BIT) { 72 has_alpha = true; 73 } 74 } 75 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8L) && !has_size) { 76+ enforce(size >= 5, Exiv2::kerCorruptedMetadata); 77 has_size = true; 78 byte size_buf_w[2]; 79 byte size_buf_h[3]; 80@@ -276,11 +287,13 @@ namespace Exiv2 { 81 82 /* Chunk with animation frame. */ 83 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_ANMF) && !has_alpha) { 84+ enforce(size >= 6, Exiv2::kerCorruptedMetadata); 85 if ((payload.pData_[5] & 0x2) == 0x2) { 86 has_alpha = true; 87 } 88 } 89 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_ANMF) && !has_size) { 90+ enforce(size >= 12, Exiv2::kerCorruptedMetadata); 91 has_size = true; 92 byte size_buf[WEBP_TAG_SIZE]; 93 94@@ -309,16 +322,22 @@ namespace Exiv2 { 95 96 io_->seek(12, BasicIo::beg); 97 while ( !io_->eof() && (uint64_t) io_->tell() < filesize) { 98- io_->read(chunkId.pData_, 4); 99- io_->read(size_buff, 4); 100+ readOrThrow(*io_, chunkId.pData_, 4, Exiv2::kerCorruptedMetadata); 101+ readOrThrow(*io_, size_buff, 4, Exiv2::kerCorruptedMetadata); 102+ 103+ const uint32_t size_u32 = Exiv2::getULong(size_buff, littleEndian); 104 105- long size = Exiv2::getULong(size_buff, littleEndian); 106+ // Check that `size_u32` is safe to cast to `long`. 107+ enforce(size_u32 <= static_cast<size_t>(std::numeric_limits<unsigned int>::max()), 108+ Exiv2::kerCorruptedMetadata); 109+ const long size = static_cast<long>(size_u32); 110 111 DataBuf payload(size); 112- io_->read(payload.pData_, size); 113+ readOrThrow(*io_, payload.pData_, size, Exiv2::kerCorruptedMetadata); 114 if ( io_->tell() % 2 ) io_->seek(+1,BasicIo::cur); // skip pad 115 116 if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8X)) { 117+ enforce(size >= 1, Exiv2::kerCorruptedMetadata); 118 if (has_icc){ 119 payload.pData_[0] |= WEBP_VP8X_ICC_BIT; 120 } else { 121