Merge pull request #4504 from hugovk/sgi_fixes

Fix buffer overflow in SGI-RLE decoding
python-pillow · Apr 1, 2020 · 2ef59fd · 2ef59fd
2 parents 0da1eca + 44096ad
commit 2ef59fd
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 2 deletions.
diff --git a/Tests/images/sgi_crash.bin b/Tests/images/sgi_crash.bin
diff --git a/Tests/images/sgi_overrun_expandrowF04.bin b/Tests/images/sgi_overrun_expandrowF04.bin
diff --git a/Tests/test_sgi_crash.py b/Tests/test_sgi_crash.py
@@ -0,0 +1,14 @@
+#!/usr/bin/env python
+import pytest
+from PIL import Image
+
+
+@pytest.mark.parametrize(
+    "test_file",
+    ["Tests/images/sgi_overrun_expandrowF04.bin", "Tests/images/sgi_crash.bin"],
+)
+def test_crashes(test_file):
+    with open(test_file, "rb") as f:
+        im = Image.open(f)
+        with pytest.raises(IOError):
+            im.load()
diff --git a/src/libImaging/SgiRleDecode.c b/src/libImaging/SgiRleDecode.c
@@ -28,6 +28,7 @@ static void read4B(UINT32* dest, UINT8* buf)
 static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
 {
     UINT8 pixel, count;
+    int x = 0;
 
     for (;n > 0; n--)
     {
@@ -37,9 +38,10 @@ static int expandrow(UINT8* dest, UINT8* src, int n, int z, int xsize)
         count = pixel & RLE_MAX_RUN;
         if (!count)
             return count;
-        if (count > xsize) {
+        if (x + count > xsize) {
             return -1;
         }
+        x += count;
         if (pixel & RLE_COPY_FLAG) {
             while(count--) {
                 *dest = *src++;
@@ -63,6 +65,7 @@ static int expandrow2(UINT8* dest, const UINT8* src, int n, int z, int xsize)
 {
     UINT8 pixel, count;
 
+    int x = 0;
 
     for (;n > 0; n--)
     {
@@ -73,9 +76,10 @@ static int expandrow2(UINT8* dest, const UINT8* src, int n, int z, int xsize)
         count = pixel & RLE_MAX_RUN;
         if (!count)
             return count;
-        if (count > xsize) {
+        if (x + count > xsize) {
             return -1;
         }
+        x += count;
         if (pixel & RLE_COPY_FLAG) {
             while(count--) {
                 memcpy(dest, src, 2);