Skip to content

Commit 4853e52

Browse files
wiredfoolradarhere
authored andcommitted
Fix OOB read in SgiRleDecode.c
* From Pillow 4.3.0->8.1.0 * CVE-2021-25293
1 parent 3fee28e commit 4853e52

9 files changed

+84
-14
lines changed
13.4 KB
Binary file not shown.
12.5 KB
Binary file not shown.
549 Bytes
Binary file not shown.
20.5 KB
Binary file not shown.
17.9 KB
Binary file not shown.
12.4 KB
Binary file not shown.
12.4 KB
Binary file not shown.

Tests/test_sgi_crash.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111
"Tests/images/sgi_crash.bin",
1212
"Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi",
1313
"Tests/images/ossfuzz-5730089102868480.sgi",
14+
"Tests/images/crash-754d9c7ec485ffb76a90eeaab191ef69a2a3a3cd.sgi",
15+
"Tests/images/crash-465703f71a0f0094873a3e0e82c9f798161171b8.sgi",
16+
"Tests/images/crash-64834657ee604b8797bf99eac6a194c124a9a8ba.sgi",
17+
"Tests/images/crash-abcf1c97b8fe42a6c68f1fb0b978530c98d57ced.sgi",
18+
"Tests/images/crash-b82e64d4f3f76d7465b6af535283029eda211259.sgi",
19+
"Tests/images/crash-c1b2595b8b0b92cc5f38b6635e98e3a119ade807.sgi",
20+
"Tests/images/crash-db8bfa78b19721225425530c5946217720d7df4e.sgi",
1421
],
1522
)
1623
def test_crashes(test_file):

src/libImaging/SgiRleDecode.c

Lines changed: 77 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,58 @@ read4B(UINT32 *dest, UINT8 *buf) {
2525
*dest = (UINT32)((buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3]);
2626
}
2727

28+
/*
29+
SgiRleDecoding is done in a single channel row oriented set of RLE chunks.
30+
31+
* The file is arranged as
32+
- SGI Header
33+
- Rle Offset Table
34+
- Rle Length Table
35+
- Scanline Data
36+
37+
* Each RLE atom is c->bpc bytes wide (1 or 2)
38+
39+
* Each RLE Chunk is [specifier atom] [ 1 or n data atoms ]
40+
41+
* Copy Atoms are a byte with the high bit set, and the low 7 are
42+
the number of bytes to copy from the source to the
43+
destination. e.g.
44+
45+
CBBBBBBBB or 0CHLHLHLHLHLHL (B=byte, H/L = Hi low bytes)
46+
47+
* Run atoms do not have the high bit set, and the low 7 bits are
48+
the number of copies of the next atom to copy to the
49+
destination. e.g.:
50+
51+
RB -> BBBBB or RHL -> HLHLHLHLHL
52+
53+
The upshot of this is, there is no way to determine the required
54+
length of the input buffer from reloffset and rlelength without
55+
going through the data at that scan line.
56+
57+
Furthermore, there's no requirement that individual scan lines
58+
pointed to from the rleoffset table are in any sort of order or
59+
used only once, or even disjoint. There's also no requirement that
60+
all of the data in the scan line area of the image file be used
61+
62+
*/
2863
static int
29-
expandrow(UINT8 *dest, UINT8 *src, int n, int z, int xsize) {
64+
expandrow(UINT8 *dest, UINT8 *src, int n, int z, int xsize, UINT8 *end_of_buffer) {
65+
/*
66+
* n here is the number of rlechunks
67+
* z is the number of channels, for calculating the interleave
68+
* offset to go to RGBA style pixels
69+
* xsize is the row width
70+
* end_of_buffer is the address of the end of the input buffer
71+
*/
72+
3073
UINT8 pixel, count;
3174
int x = 0;
3275

3376
for (; n > 0; n--) {
77+
if (src > end_of_buffer) {
78+
return -1;
79+
}
3480
pixel = *src++;
3581
if (n == 1 && pixel != 0) {
3682
return n;
@@ -44,12 +90,18 @@ expandrow(UINT8 *dest, UINT8 *src, int n, int z, int xsize) {
4490
}
4591
x += count;
4692
if (pixel & RLE_COPY_FLAG) {
93+
if (src + count > end_of_buffer) {
94+
return -1;
95+
}
4796
while (count--) {
4897
*dest = *src++;
4998
dest += z;
5099
}
51100

52101
} else {
102+
if (src > end_of_buffer) {
103+
return -1;
104+
}
53105
pixel = *src++;
54106
while (count--) {
55107
*dest = pixel;
@@ -61,12 +113,14 @@ expandrow(UINT8 *dest, UINT8 *src, int n, int z, int xsize) {
61113
}
62114

63115
static int
64-
expandrow2(UINT8 *dest, const UINT8 *src, int n, int z, int xsize) {
116+
expandrow2(UINT8 *dest, const UINT8 *src, int n, int z, int xsize, UINT8 *end_of_buffer) {
65117
UINT8 pixel, count;
66-
67118
int x = 0;
68119

69120
for (; n > 0; n--) {
121+
if (src + 1 > end_of_buffer) {
122+
return -1;
123+
}
70124
pixel = src[1];
71125
src += 2;
72126
if (n == 1 && pixel != 0) {
@@ -81,12 +135,18 @@ expandrow2(UINT8 *dest, const UINT8 *src, int n, int z, int xsize) {
81135
}
82136
x += count;
83137
if (pixel & RLE_COPY_FLAG) {
138+
if (src + 2 * count > end_of_buffer) {
139+
return -1;
140+
}
84141
while (count--) {
85142
memcpy(dest, src, 2);
86143
src += 2;
87144
dest += z * 2;
88145
}
89146
} else {
147+
if (src + 2 > end_of_buffer) {
148+
return -1;
149+
}
90150
while (count--) {
91151
memcpy(dest, src, 2);
92152
dest += z * 2;
@@ -132,7 +192,11 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t
132192
return -1;
133193
}
134194
_imaging_seek_pyFd(state->fd, SGI_HEADER_SIZE, SEEK_SET);
135-
_imaging_read_pyFd(state->fd, (char *)ptr, c->bufsize);
195+
if (_imaging_read_pyFd(state->fd, (char *)ptr, c->bufsize) != c->bufsize) {
196+
state->errcode = IMAGING_CODEC_UNKNOWN;
197+
return -1;
198+
}
199+
136200

137201
/* decoder initialization */
138202
state->count = 0;
@@ -166,35 +230,37 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t
166230
read4B(&c->lengthtab[c->tabindex], &ptr[c->bufindex]);
167231
}
168232

169-
state->count += c->tablen * sizeof(UINT32) * 2;
170-
171233
/* read compressed rows */
172234
for (c->rowno = 0; c->rowno < im->ysize; c->rowno++, state->y += state->ystep) {
173235
for (c->channo = 0; c->channo < im->bands; c->channo++) {
174236
c->rleoffset = c->starttab[c->rowno + c->channo * im->ysize];
175237
c->rlelength = c->lengthtab[c->rowno + c->channo * im->ysize];
176-
c->rleoffset -= SGI_HEADER_SIZE;
177238

178-
if (c->rleoffset + c->rlelength > c->bufsize) {
239+
// Check for underflow of rleoffset-SGI_HEADER_SIZE
240+
if (c->rleoffset < SGI_HEADER_SIZE) {
179241
state->errcode = IMAGING_CODEC_OVERRUN;
180242
goto sgi_finish_decode;
181243
}
182244

245+
c->rleoffset -= SGI_HEADER_SIZE;
246+
183247
/* row decompression */
184248
if (c->bpc == 1) {
185249
status = expandrow(
186250
&state->buffer[c->channo],
187251
&ptr[c->rleoffset],
188252
c->rlelength,
189253
im->bands,
190-
im->xsize);
254+
im->xsize,
255+
&ptr[c->bufsize-1]);
191256
} else {
192257
status = expandrow2(
193258
&state->buffer[c->channo * 2],
194259
&ptr[c->rleoffset],
195260
c->rlelength,
196261
im->bands,
197-
im->xsize);
262+
im->xsize,
263+
&ptr[c->bufsize-1]);
198264
}
199265
if (status == -1) {
200266
state->errcode = IMAGING_CODEC_OVERRUN;
@@ -203,15 +269,12 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t
203269
goto sgi_finish_decode;
204270
}
205271

206-
state->count += c->rlelength;
207272
}
208273

209274
/* store decompressed data in image */
210275
state->shuffle((UINT8 *)im->image[state->y], state->buffer, im->xsize);
211276
}
212277

213-
c->bufsize++;
214-
215278
sgi_finish_decode:;
216279

217280
free(c->starttab);
@@ -221,5 +284,5 @@ sgi_finish_decode:;
221284
state->errcode = err;
222285
return -1;
223286
}
224-
return state->count - c->bufsize;
287+
return 0;
225288
}

0 commit comments

Comments
 (0)