From 783b3a6ff15ed6f82a8f8e6c8a6f3b84a9b04d4b Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 19 Apr 2021 18:06:00 +0100 Subject: [PATCH] Improve bound checking in WebPImage::doWriteMetadata() --- src/webpimage.cpp | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/webpimage.cpp b/src/webpimage.cpp index 4ddec544c..fee110bca 100644 --- a/src/webpimage.cpp +++ b/src/webpimage.cpp @@ -145,7 +145,7 @@ namespace Exiv2 { DataBuf chunkId(WEBP_TAG_SIZE+1); chunkId.pData_ [WEBP_TAG_SIZE] = '\0'; - io_->read(data, WEBP_TAG_SIZE * 3); + readOrThrow(*io_, data, WEBP_TAG_SIZE * 3, Exiv2::kerCorruptedMetadata); uint64_t filesize = Exiv2::getULong(data + WEBP_TAG_SIZE, littleEndian); /* Set up header */ @@ -185,13 +185,20 @@ namespace Exiv2 { case we have any exif or xmp data, also check for any chunks with alpha frame/layer set */ while ( !io_->eof() && (uint64_t) io_->tell() < filesize) { - io_->read(chunkId.pData_, WEBP_TAG_SIZE); - io_->read(size_buff, WEBP_TAG_SIZE); - long size = Exiv2::getULong(size_buff, littleEndian); + readOrThrow(*io_, chunkId.pData_, WEBP_TAG_SIZE, Exiv2::kerCorruptedMetadata); + readOrThrow(*io_, size_buff, WEBP_TAG_SIZE, Exiv2::kerCorruptedMetadata); + const uint32_t size_u32 = Exiv2::getULong(size_buff, littleEndian); + + // Check that `size_u32` is safe to cast to `long`. + enforce(size_u32 <= static_cast(std::numeric_limits::max()), + Exiv2::kerCorruptedMetadata); + const long size = static_cast(size_u32); DataBuf payload(size); - io_->read(payload.pData_, payload.size_); - byte c; - if ( payload.size_ % 2 ) io_->read(&c,1); + readOrThrow(*io_, payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); + if ( payload.size_ % 2 ) { + byte c; + readOrThrow(*io_, &c, 1, Exiv2::kerCorruptedMetadata); + } /* Chunk with information about features used in the file. */ @@ -199,6 +206,7 @@ namespace Exiv2 { has_vp8x = true; } if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8X) && !has_size) { + enforce(size >= 10, Exiv2::kerCorruptedMetadata); has_size = true; byte size_buf[WEBP_TAG_SIZE]; @@ -227,6 +235,7 @@ namespace Exiv2 { } #endif if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8) && !has_size) { + enforce(size >= 10, Exiv2::kerCorruptedMetadata); has_size = true; byte size_buf[2]; @@ -244,11 +253,13 @@ namespace Exiv2 { /* Chunk with with lossless image data. */ if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8L) && !has_alpha) { + enforce(size >= 5, Exiv2::kerCorruptedMetadata); if ((payload.pData_[4] & WEBP_VP8X_ALPHA_BIT) == WEBP_VP8X_ALPHA_BIT) { has_alpha = true; } } if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8L) && !has_size) { + enforce(size >= 5, Exiv2::kerCorruptedMetadata); has_size = true; byte size_buf_w[2]; byte size_buf_h[3]; @@ -276,11 +287,13 @@ namespace Exiv2 { /* Chunk with animation frame. */ if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_ANMF) && !has_alpha) { + enforce(size >= 6, Exiv2::kerCorruptedMetadata); if ((payload.pData_[5] & 0x2) == 0x2) { has_alpha = true; } } if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_ANMF) && !has_size) { + enforce(size >= 12, Exiv2::kerCorruptedMetadata); has_size = true; byte size_buf[WEBP_TAG_SIZE]; @@ -309,16 +322,22 @@ namespace Exiv2 { io_->seek(12, BasicIo::beg); while ( !io_->eof() && (uint64_t) io_->tell() < filesize) { - io_->read(chunkId.pData_, 4); - io_->read(size_buff, 4); + readOrThrow(*io_, chunkId.pData_, 4, Exiv2::kerCorruptedMetadata); + readOrThrow(*io_, size_buff, 4, Exiv2::kerCorruptedMetadata); + + const uint32_t size_u32 = Exiv2::getULong(size_buff, littleEndian); - long size = Exiv2::getULong(size_buff, littleEndian); + // Check that `size_u32` is safe to cast to `long`. + enforce(size_u32 <= static_cast(std::numeric_limits::max()), + Exiv2::kerCorruptedMetadata); + const long size = static_cast(size_u32); DataBuf payload(size); - io_->read(payload.pData_, size); + readOrThrow(*io_, payload.pData_, size, Exiv2::kerCorruptedMetadata); if ( io_->tell() % 2 ) io_->seek(+1,BasicIo::cur); // skip pad if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_VP8X)) { + enforce(size >= 1, Exiv2::kerCorruptedMetadata); if (has_icc){ payload.pData_[0] |= WEBP_VP8X_ICC_BIT; } else {