1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
|
From 783b3a6ff15ed6f82a8f8e6c8a6f3b84a9b04d4b Mon Sep 17 00:00:00 2001
From: Kevin Backhouse <kevinbackhouse@github.com>
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<size_t>(std::numeric_limits<unsigned int>::max()),
+ Exiv2::kerCorruptedMetadata);
+ const long size = static_cast<long>(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<size_t>(std::numeric_limits<unsigned int>::max()),
+ Exiv2::kerCorruptedMetadata);
+ const long size = static_cast<long>(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 {
|
