File 016-Fix-OOB-read-in-SgiRleDecode.patch of Package python-Pillow
From f891baa604636cd2506a9360d170bc2cf4963cc5 Mon Sep 17 00:00:00 2001
From: Eric Soroos <eric-github@soroos.net>
Date: Sat, 2 Jan 2021 16:07:36 +0100
Subject: [PATCH] Fix OOB read in SgiRleDecode.c
* From Pillow 4.3.0->8.1.0
* CVE-2021-25293
---
...5703f71a0f0094873a3e0e82c9f798161171b8.sgi | Bin 0 -> 13703 bytes
...834657ee604b8797bf99eac6a194c124a9a8ba.sgi | Bin 0 -> 12789 bytes
...4d9c7ec485ffb76a90eeaab191ef69a2a3a3cd.sgi | Bin 0 -> 549 bytes
...cf1c97b8fe42a6c68f1fb0b978530c98d57ced.sgi | Bin 0 -> 21017 bytes
...2e64d4f3f76d7465b6af535283029eda211259.sgi | Bin 0 -> 18364 bytes
...b2595b8b0b92cc5f38b6635e98e3a119ade807.sgi | Bin 0 -> 12748 bytes
...8bfa78b19721225425530c5946217720d7df4e.sgi | Bin 0 -> 12744 bytes
Tests/test_sgi_crash.py | 7 ++
src/libImaging/SgiRleDecode.c | 88 +++++++++++++++---
9 files changed, 81 insertions(+), 14 deletions(-)
create mode 100644 Tests/images/crash-465703f71a0f0094873a3e0e82c9f798161171b8.sgi
create mode 100644 Tests/images/crash-64834657ee604b8797bf99eac6a194c124a9a8ba.sgi
create mode 100644 Tests/images/crash-754d9c7ec485ffb76a90eeaab191ef69a2a3a3cd.sgi
create mode 100644 Tests/images/crash-abcf1c97b8fe42a6c68f1fb0b978530c98d57ced.sgi
create mode 100644 Tests/images/crash-b82e64d4f3f76d7465b6af535283029eda211259.sgi
create mode 100644 Tests/images/crash-c1b2595b8b0b92cc5f38b6635e98e3a119ade807.sgi
create mode 100644 Tests/images/crash-db8bfa78b19721225425530c5946217720d7df4e.sgi
diff --git a/Tests/test_sgi_crash.py b/Tests/test_sgi_crash.py
index ac304aab4d..d4ddc12f9f 100644
--- a/Tests/test_sgi_crash.py
+++ b/Tests/test_sgi_crash.py
@@ -10,6 +10,13 @@
"Tests/images/sgi_crash.bin",
"Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi",
"Tests/images/ossfuzz-5730089102868480.sgi",
+ "Tests/images/crash-754d9c7ec485ffb76a90eeaab191ef69a2a3a3cd.sgi",
+ "Tests/images/crash-465703f71a0f0094873a3e0e82c9f798161171b8.sgi",
+ "Tests/images/crash-64834657ee604b8797bf99eac6a194c124a9a8ba.sgi",
+ "Tests/images/crash-abcf1c97b8fe42a6c68f1fb0b978530c98d57ced.sgi",
+ "Tests/images/crash-b82e64d4f3f76d7465b6af535283029eda211259.sgi",
+ "Tests/images/crash-c1b2595b8b0b92cc5f38b6635e98e3a119ade807.sgi",
+ "Tests/images/crash-db8bfa78b19721225425530c5946217720d7df4e.sgi",
],
)
def test_crashes(test_file):
diff --git a/src/libImaging/SgiRleDecode.c b/src/libImaging/SgiRleDecode.c
index 9a8814b50c..2afa9a6f99 100644
--- a/src/libImaging/SgiRleDecode.c
+++ b/src/libImaging/SgiRleDecode.c
@@ -25,13 +25,59 @@ static void read4B(UINT32* dest, UINT8* buf)
*dest = (UINT32)((buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3]);
}
-static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
+/*
+ SgiRleDecoding is done in a single channel row oriented set of RLE chunks.
+
+ * The file is arranged as
+ - SGI Header
+ - Rle Offset Table
+ - Rle Length Table
+ - Scanline Data
+
+ * Each RLE atom is c->bpc bytes wide (1 or 2)
+
+ * Each RLE Chunk is [specifier atom] [ 1 or n data atoms ]
+
+ * Copy Atoms are a byte with the high bit set, and the low 7 are
+ the number of bytes to copy from the source to the
+ destination. e.g.
+
+ CBBBBBBBB or 0CHLHLHLHLHLHL (B=byte, H/L = Hi low bytes)
+
+ * Run atoms do not have the high bit set, and the low 7 bits are
+ the number of copies of the next atom to copy to the
+ destination. e.g.:
+
+ RB -> BBBBB or RHL -> HLHLHLHLHL
+
+ The upshot of this is, there is no way to determine the required
+ length of the input buffer from reloffset and rlelength without
+ going through the data at that scan line.
+
+ Furthermore, there's no requirement that individual scan lines
+ pointed to from the rleoffset table are in any sort of order or
+ used only once, or even disjoint. There's also no requirement that
+ all of the data in the scan line area of the image file be used
+
+ */
+static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize, UINT8* end_of_buffer)
{
+ /*
+ * n here is the number of rlechunks
+ * z is the number of channels, for calculating the interleave
+ * offset to go to RGBA style pixels
+ * xsize is the row width
+ * end_of_buffer is the address of the end of the input buffer
+ */
+
UINT8 pixel, count;
int x = 0;
for (;n > 0; n--)
{
+ if (src > end_of_buffer) {
+ return -1;
+ }
pixel = *src++;
if (n == 1 && pixel != 0)
return n;
@@ -43,6 +89,9 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
}
x += count;
if (pixel & RLE_COPY_FLAG) {
+ if (src + count > end_of_buffer) {
+ return -1;
+ }
while(count--) {
*dest = *src++;
dest += z;
@@ -50,6 +99,9 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
}
else {
+ if (src > end_of_buffer) {
+ return -1;
+ }
pixel = *src++;
while (count--) {
*dest = pixel;
@@ -61,14 +113,16 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
return 0;
}
-static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize)
+static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize, UINT8* end_of_buffer)
{
UINT8 pixel, count;
-
int x = 0;
for (;n > 0; n--)
{
+ if ((UINT8*)src + 1 > end_of_buffer) {
+ return -1;
+ }
pixel = ((UINT8*)src)[1];
++src;
if (n == 1 && pixel != 0)
@@ -81,6 +135,9 @@ static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize)
}
x += count;
if (pixel & RLE_COPY_FLAG) {
+ if ((UINT8*)src + 2 * count > end_of_buffer) {
+ return -1;
+ }
while(count--) {
*dest = *src++;
dest += z;
@@ -87,6 +144,9 @@ static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize)
}
}
else {
+ if ((UINT8*)src + 2 > end_of_buffer) {
+ return -1;
+ }
while (count--) {
*dest = *src;
dest += z;
@@ -136,7 +196,10 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
return -1;
}
_imaging_seek_pyFd(state->fd, SGI_HEADER_SIZE, SEEK_SET);
- _imaging_read_pyFd(state->fd, (char*)ptr, c->bufsize);
+ if (_imaging_read_pyFd(state->fd, (char*)ptr, c->bufsize) != c->bufsize) {
+ state->errcode = IMAGING_CODEC_UNKNOWN;
+ return -1;
+ }
/* decoder initialization */
@@ -168,8 +231,6 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
for (c->tabindex = 0, c->bufindex = c->tablen * sizeof(UINT32); c->tabindex < c->tablen; c->tabindex++, c->bufindex+=4)
read4B(&c->lengthtab[c->tabindex], &ptr[c->bufindex]);
- state->count += c->tablen * sizeof(UINT32) * 2;
-
/* read compressed rows */
for (c->rowno = 0; c->rowno < im->ysize; c->rowno++, state->y += state->ystep)
{
@@ -177,19 +242,21 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
{
c->rleoffset = c->starttab[c->rowno + c->channo * im->ysize];
c->rlelength = c->lengthtab[c->rowno + c->channo * im->ysize];
- c->rleoffset -= SGI_HEADER_SIZE;
- if (c->rleoffset + c->rlelength > c->bufsize) {
+ // Check for underflow of rleoffset-SGI_HEADER_SIZE
+ if (c->rleoffset < SGI_HEADER_SIZE) {
state->errcode = IMAGING_CODEC_OVERRUN;
goto sgi_finish_decode;
}
+ c->rleoffset -= SGI_HEADER_SIZE;
+
/* row decompression */
if (c->bpc ==1) {
- status = expandrow(&state->buffer[c->channo], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize);
+ status = expandrow(&state->buffer[c->channo], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize, &ptr[c->bufsize-1]);
}
else {
- status = expandrow2(&state->buffer[c->channo * 2], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize);
+ status = expandrow2(&state->buffer[c->channo * 2], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize, &ptr[c->bufsize-1]);
}
if (status == -1) {
state->errcode = IMAGING_CODEC_OVERRUN;
@@ -198,7 +261,6 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
goto sgi_finish_decode;
}
- state->count += c->rlelength;
}
/* store decompressed data in image */
@@ -206,8 +268,6 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
}
- c->bufsize++;
-
sgi_finish_decode: ;
free(c->starttab);
@@ -217,5 +277,5 @@ sgi_finish_decode: ;
state->errcode=err;
return -1;
}
- return state->count - c->bufsize;
+ return 0;
}