We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

[patch] CVE-2019-12221: patch proposal (pcx: do not write directly to row buffer)

CVE-2019-12221.patch (text/plain), 2.50 KB, created by Hugo Lefeuvre on 2019-05-25 11:32:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Hugo Lefeuvre

Created: 2019-05-25 11:32:37 UTC

Size: 2.50 KB

patch

obsolete

># HG changeset patch
># User hle@debian.org
># Date 1558783494 -7200
>#      Sat May 25 13:24:54 2019 +0200
># Node ID 4775f98ff2501e7a3a28d5cd87031398ef7c96fd
># Parent  61f1bf992955ba796db46f2d684a29c30e94b545
>pcx: do not write directly to row buffer
>
>The PCX format specifies pcxh.BytesPerLine, which represents the size of a
>single plane's scanline in bytes. Valid PCX images should have
>pcxh.BytesPerLine >= surface->pitch.
>
>pcxh.BytesPerLine and surface->pitch can legitimately be different because
>pcxh.BytesPerLine is padded to be a multiple of machine word length (where
>file was created).
>
>If src_bits == 8 we directly read a whole scanline from src to row. This is
>a problem in the case where bpl > surface->pitch because row is too small.
>
>This allows attacker to perform unlimited OOB write on the heap.
>
>+ remove pointless check bpl > surface->pitch, this is a valid situation
>+ make sure we always read into buf which is big enough
>+ in the case where src_bits == 8: copy these bytes back to row afterwards
>
>diff -r 61f1bf992955 -r 4775f98ff250 IMG_pcx.c
>--- a/IMG_pcx.c	Fri Apr 12 16:29:43 2019 -0400
>+++ b/IMG_pcx.c	Sat May 25 13:24:54 2019 +0200
>@@ -144,18 +144,15 @@
>         goto done;
> 
>     bpl = pcxh.NPlanes * pcxh.BytesPerLine;
>-    if (bpl > surface->pitch) {
>-        error = "bytes per line is too large (corrupt?)";
>-    }
>     buf = (Uint8 *)SDL_calloc(SDL_max(bpl, surface->pitch), 1);
>     row = (Uint8 *)surface->pixels;
>     for ( y=0; y<surface->h; ++y ) {
>         /* decode a scan line to a temporary buffer first */
>         int i, count = 0;
>         Uint8 ch;
>-        Uint8 *dst = (src_bits == 8) ? row : buf;
>+        Uint8 *dst;
>         if ( pcxh.Encoding == 0 ) {
>-            if(!SDL_RWread(src, dst, bpl, 1)) {
>+            if(!SDL_RWread(src, buf, bpl, 1)) {
>                 error = "file truncated";
>                 goto done;
>             }
>@@ -175,7 +172,7 @@
>                     } else
>                         count = 1;
>                 }
>-                dst[i] = ch;
>+                buf[i] = ch;
>                 count--;
>             }
>         }
>@@ -197,6 +194,14 @@
>                     }
>                 }
>             }
>+        } else if(src_bits == 8) {
>+            /* directly copy buf content to row */
>+            Uint8 *innerSrc = buf;
>+            int x;
>+            dst = row;
>+            for(x = 0; x < width; x++) {
>+                *dst++ = *innerSrc++;
>+            }
>         } else if(src_bits == 24) {
>             /* de-interlace planes */
>             Uint8 *innerSrc = buf;

Actions: View | Diff

Attachments on bug 4628: 3780 | 3797

We are currently migrating Bugzilla to GitHub issues.Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.