ed67fe
From 1312c5426e7dd84e396ef2ff35aa09b64d92d382 Mon Sep 17 00:00:00 2001
ed67fe
From: Lumir Balhar <lbalhar@redhat.com>
ed67fe
Date: Fri, 9 Apr 2021 19:33:55 +0200
ed67fe
Subject: [PATCH 3/4] CVE-2021-25293
ed67fe
ed67fe
---
ed67fe
 src/libImaging/SgiRleDecode.c | 88 +++++++++++++++++++++++++++++------
ed67fe
 1 file changed, 75 insertions(+), 13 deletions(-)
ed67fe
ed67fe
diff --git a/src/libImaging/SgiRleDecode.c b/src/libImaging/SgiRleDecode.c
ed67fe
index 2259159..85af456 100644
ed67fe
--- a/src/libImaging/SgiRleDecode.c
ed67fe
+++ b/src/libImaging/SgiRleDecode.c
ed67fe
@@ -25,13 +25,60 @@ static void read4B(UINT32* dest, UINT8* buf)
ed67fe
     *dest = (UINT32)((buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3]);
ed67fe
 }
ed67fe
 
ed67fe
-static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
ed67fe
+/*
ed67fe
+   SgiRleDecoding is done in a single channel row oriented set of RLE chunks.
ed67fe
+
ed67fe
+   * The file is arranged as
ed67fe
+     - SGI Header
ed67fe
+     - Rle Offset Table
ed67fe
+     - Rle Length Table
ed67fe
+     - Scanline Data
ed67fe
+
ed67fe
+   * Each RLE atom is c->bpc bytes wide (1 or 2)
ed67fe
+
ed67fe
+   * Each RLE Chunk is [specifier atom] [ 1 or n data atoms ]
ed67fe
+
ed67fe
+   * Copy Atoms are a byte with the high bit set, and the low 7 are
ed67fe
+     the number of bytes to copy from the source to the
ed67fe
+     destination. e.g.
ed67fe
+
ed67fe
+         CBBBBBBBB or 0CHLHLHLHLHLHL   (B=byte, H/L = Hi low bytes)
ed67fe
+
ed67fe
+   * Run atoms do not have the high bit set, and the low 7 bits are
ed67fe
+     the number of copies of the next atom to copy to the
ed67fe
+     destination. e.g.:
ed67fe
+
ed67fe
+         RB -> BBBBB or RHL -> HLHLHLHLHL
ed67fe
+
ed67fe
+   The upshot of this is, there is no way to determine the required
ed67fe
+   length of the input buffer from reloffset and rlelength without
ed67fe
+   going through the data at that scan line.
ed67fe
+
ed67fe
+   Furthermore, there's no requirement that individual scan lines
ed67fe
+   pointed to from the rleoffset table are in any sort of order or
ed67fe
+   used only once, or even disjoint. There's also no requirement that
ed67fe
+   all of the data in the scan line area of the image file be used
ed67fe
+
ed67fe
+ */
ed67fe
+
ed67fe
+static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize, UINT8 *end_of_buffer)
ed67fe
 {
ed67fe
+    /*
ed67fe
+     * n here is the number of rlechunks
ed67fe
+     * z is the number of channels, for calculating the interleave
ed67fe
+     *   offset to go to RGBA style pixels
ed67fe
+     * xsize is the row width
ed67fe
+     * end_of_buffer is the address of the end of the input buffer
ed67fe
+     */
ed67fe
+
ed67fe
     UINT8 pixel, count;
ed67fe
     int x = 0;
ed67fe
 
ed67fe
     for (;n > 0; n--)
ed67fe
     {
ed67fe
+        if (src > end_of_buffer) {
ed67fe
+            return -1;
ed67fe
+        }
ed67fe
         pixel = *src++;
ed67fe
         if (n == 1 && pixel != 0)
ed67fe
             return n;
ed67fe
@@ -43,6 +90,9 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
ed67fe
         }
ed67fe
         x += count;
ed67fe
         if (pixel & RLE_COPY_FLAG) {
ed67fe
+            if (src + count > end_of_buffer) {
ed67fe
+                return -1;
ed67fe
+            }
ed67fe
             while(count--) {
ed67fe
                 *dest = *src++;
ed67fe
                 dest += z;
ed67fe
@@ -50,6 +100,9 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
ed67fe
 
ed67fe
         }
ed67fe
         else {
ed67fe
+            if (src > end_of_buffer) {
ed67fe
+                return -1;
ed67fe
+            }
ed67fe
             pixel = *src++;
ed67fe
             while (count--) {
ed67fe
                 *dest = pixel;
ed67fe
@@ -61,7 +114,7 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
ed67fe
     return 0;
ed67fe
 }
ed67fe
 
ed67fe
-static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize)
ed67fe
+static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize, UINT8 *end_of_buffer)
ed67fe
 {
ed67fe
     UINT8 pixel, count;
ed67fe
 
ed67fe
@@ -69,6 +122,9 @@ static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize)
ed67fe
 
ed67fe
     for (;n > 0; n--)
ed67fe
     {
ed67fe
+        if (src + 1 > end_of_buffer) {
ed67fe
+            return -1;
ed67fe
+        }
ed67fe
         pixel = ((UINT8*)src)[1];
ed67fe
         ++src;
ed67fe
         if (n == 1 && pixel != 0)
ed67fe
@@ -81,12 +137,18 @@ static int expandrow2(UINT16* dest, UINT16* src, int n, int z, int xsize)
ed67fe
         }
ed67fe
         x += count;
ed67fe
         if (pixel & RLE_COPY_FLAG) {
ed67fe
+            if (src + 2 * count > end_of_buffer) {
ed67fe
+                return -1;
ed67fe
+            }
ed67fe
             while(count--) {
ed67fe
                 *dest = *src++;
ed67fe
                 dest += z;
ed67fe
             }
ed67fe
         }
ed67fe
         else {
ed67fe
+            if (src + 2 > end_of_buffer) {
ed67fe
+                return -1;
ed67fe
+            }
ed67fe
             while (count--) {
ed67fe
                 *dest = *src;
ed67fe
                 dest += z;
ed67fe
@@ -136,8 +198,10 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
ed67fe
         return -1;
ed67fe
     }
ed67fe
     _imaging_seek_pyFd(state->fd, SGI_HEADER_SIZE, SEEK_SET);
ed67fe
-    _imaging_read_pyFd(state->fd, (char*)ptr, c->bufsize);
ed67fe
-
ed67fe
+    if (_imaging_read_pyFd(state->fd, (char *)ptr, c->bufsize) != c->bufsize) {
ed67fe
+        state->errcode = IMAGING_CODEC_UNKNOWN;
ed67fe
+        return -1;
ed67fe
+    }
ed67fe
 
ed67fe
     /* decoder initialization */
ed67fe
     state->count = 0;
ed67fe
@@ -168,8 +232,6 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
ed67fe
     for (c->tabindex = 0, c->bufindex = c->tablen * sizeof(UINT32); c->tabindex < c->tablen; c->tabindex++, c->bufindex+=4)
ed67fe
         read4B(&c->lengthtab[c->tabindex], &ptr[c->bufindex]);
ed67fe
 
ed67fe
-    state->count += c->tablen * sizeof(UINT32) * 2;
ed67fe
-
ed67fe
     /* read compressed rows */
ed67fe
     for (c->rowno = 0; c->rowno < im->ysize; c->rowno++, state->y += state->ystep)
ed67fe
     {
ed67fe
@@ -177,19 +239,21 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
ed67fe
         {
ed67fe
             c->rleoffset = c->starttab[c->rowno + c->channo * im->ysize];
ed67fe
             c->rlelength = c->lengthtab[c->rowno + c->channo * im->ysize];
ed67fe
-            c->rleoffset -= SGI_HEADER_SIZE;
ed67fe
 
ed67fe
-            if (c->rleoffset + c->rlelength > c->bufsize) {
ed67fe
+            // Check for underflow of rleoffset-SGI_HEADER_SIZE
ed67fe
+            if (c->rleoffset < SGI_HEADER_SIZE) {
ed67fe
                 state->errcode = IMAGING_CODEC_OVERRUN;
ed67fe
                 goto sgi_finish_decode;
ed67fe
             }
ed67fe
 
ed67fe
+            c->rleoffset -= SGI_HEADER_SIZE;
ed67fe
+
ed67fe
             /* row decompression */
ed67fe
             if (c->bpc ==1) {
ed67fe
-                status = expandrow(&state->buffer[c->channo], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize);
ed67fe
+                status = expandrow(&state->buffer[c->channo], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize, &ptr[c->bufsize-1]);
ed67fe
             }
ed67fe
             else {
ed67fe
-                status = expandrow2(&state->buffer[c->channo * 2], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize);
ed67fe
+                status = expandrow2(&state->buffer[c->channo * 2], &ptr[c->rleoffset], c->rlelength, im->bands, im->xsize, &ptr[c->bufsize-1]);
ed67fe
             }
ed67fe
             if (status == -1) {
ed67fe
                 state->errcode = IMAGING_CODEC_OVERRUN;
ed67fe
@@ -198,7 +262,6 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
ed67fe
                 goto sgi_finish_decode;
ed67fe
             }
ed67fe
 
ed67fe
-            state->count += c->rlelength;
ed67fe
         }
ed67fe
 
ed67fe
         /* store decompressed data in image */
ed67fe
@@ -206,7 +269,6 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state,
ed67fe
 
ed67fe
     }
ed67fe
 
ed67fe
-    c->bufsize++;
ed67fe
 
ed67fe
 sgi_finish_decode: ;
ed67fe
 
ed67fe
@@ -217,5 +279,5 @@ sgi_finish_decode: ;
ed67fe
         state->errcode=err;
ed67fe
         return -1;
ed67fe
     }
ed67fe
-    return state->count - c->bufsize;
ed67fe
+    return 0;
ed67fe
 }
ed67fe
-- 
ed67fe
2.30.2
ed67fe