From 7e95c63fa7f503f185d3d9eb16b9cee1e54d1e46 Mon Sep 17 00:00:00 2001 From: Eric Soroos Date: Thu, 29 Oct 2020 23:07:15 +0000 Subject: [PATCH 1/3] Fix for SGI Decode buffer overrun CVE-2020-35655 * Independently found by a contributor and sent to Tidelift, and by Google's OSS Fuzz. --- ...7f2244da6d0ae297ee0754a424213444e92778.sgi | Bin 0 -> 6973 bytes Tests/images/ossfuzz-5730089102868480.sgi | Bin 0 -> 530 bytes Tests/test_sgi_crash.py | 8 +++++- src/libImaging/SgiRleDecode.c | 23 ++++++++++++------ 4 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi create mode 100644 Tests/images/ossfuzz-5730089102868480.sgi diff --git a/Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi b/Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi new file mode 100644 index 0000000000000000000000000000000000000000..74396935b9a5df427fdcc80c5d884e0196f6caf0 GIT binary patch literal 6973 zcmeI0e^6G}na4l(_qq4I_bwXKxW*WUl9XCw(NaS-H4Y`w7*cIY3`;30MPn!-#IVG; zl%@0?N{u>7S*ylaLJ854#9RI~)^6DQ~flo{5&u)T66OghU?uv&MEwCyZzUo8n za`^focvK$W=3Mw@6MQEIc5H<2b;0+iLVXTAcNmVyd`kti&w^i$z^k3`t9}@o10%=a zUCA?%iRNrXi|It0k%bl?gLca$v}8FNDNE4OYSHdbKwI05mS2PR*nYI~HneT;q3y{= ztLsNQT!GfI60IW{t!FXXzy`FDI<%|9=vET?lxp-F#?fbQKwoeX{R=thD|*ng63{mk zq8A-PFB?SP=A-YKh5pQJ^#6`WKRy+`V-&ro7k!`^eWV`!>Rt?UD@IHi#>~wa2?ZEQ z8!(pSV5DVXWQcNCVHAo=M3pkGmbDtOIh2RdT#V6Pj&W)`#u;CpOR#P^iuJi1tUJoE?oGm49gmg266^8pSQUL()ibf4UW3(8 zfb~KNR>wxH(^*)93$ZTySbvy`ZI@$DX~CX(68jU4*z?P%@LjVUHwZzngzdfPHH#KJqvK|E5`Y%*shD>Jd}=8^d3%0InK9+ah{li zQYgHsW4?7Wbwf;(j^-H@O%$B^~$fG2Hu- zakH~c4s#@k+ix9f{|`^B$b;XOBqcSQV-@4|br z5bx#Xc&Fp>UN`ZE&f<+Un~g& zl4cS~k0p|oLL@hjNPZQOqGLo#28l%9A+k*o`L9_-o=PI}%t|89l@dA9NaXkkk&|B6_)Nmt7YXMT5Z*dMxHy|I^(Df)XA^#@ zobdi$!t7aux#@&?n+OYc5EhF{qJ-t^2`d*6ZW|^1ZVlm{g@pTC2V5M!sB}hPb3qbJVf}?6~eBogx!+AcNO7lhY8Qr5uTG+Z=@0qO5VX!gl|gTH}gfu z2#1tNY=^{d=pDjeFA;4Qogo~Sy$;LXhUMPQPZQ;cBLsT!4dl^X)?GT+Me0z>)Gt5hXTkB!*Tu5z$yEEWR??A>rSX~V3 zWv(C={=EcBdLSzHPYC>}8iu_K;puWXa0(9J49^Q#`bh{Uo1tqf^h*4*HE`aB%XM(I z08Pt5^LC(JGmdt{ZnRGbD4Mq%ZBahj9Y@ht#-n9QzfQ)5GXEdxXceVsJ5QoLH6QIj zD_T<#TH7)-ITu=A8ro0^+LdEy|0m?pO-H|0piW#Y`dk4-iv;#8twjIw74$Vr(DS3{ zn_AJMGTtd`PsO1h6kv2j0MSnsdZ&P-zE1R^M)WJS=;OOER3*l=QjA!EJah6flGkIT zuEAI-5GX4XV}k&pB2ife#@1|%-MJWb0)^xp8!ZBpIw~-FsxSujVF#;74uRi=I@KJ%xbKEX~de*gLU&%ta*E}KA(?uR}0pvDAw01u?ml2ePbN!iBzoJ zGJZzJjU8A&>B8!4!+NbAYgjOAjDqsL+rZ+F5Op& zy;^|NgCp3Fj$wb}3ih|Vv3KsnesU#t-C690h1k#U#cmtLeo6dam9;Yi*uw&zF5iiL zwGc;% z0fA}1p1}D{G0v5KoH2?22Z3Vbp{!*ndB3Vq;MLSp)O7;1uJ1v`3hcT`K-MS4=9VaG zp6IjHsLu_c7B`?$7o+YHkhP*4bzd{;D>bOKvi^YRp*qx~ZK%gbP-OzwD#}n>1*Cm< zA!?7<)|8;0mG}+ff3zOeD!wO&P_N{o`UI4{F&%ZW2{rl->irsAPe9nT5!@T&a1(lP z=cnKBfR{tMUEK4u| literal 0 HcmV?d00001 diff --git a/Tests/images/ossfuzz-5730089102868480.sgi b/Tests/images/ossfuzz-5730089102868480.sgi new file mode 100644 index 0000000000000000000000000000000000000000..a92c1ed019bf1505edc5cc7444f72bd89f7105b7 GIT binary patch literal 530 zcmbV}JxT;Y5QX0x1_l2X!3#{Vk%1bUmuz`gUnXQAvCj}~ z7>4_*w5lQ8R_)W3#s6Qn?)|gR$6YB~%3<#QnCan5OLprB9k|!1GG*%B<)Kn9rCeY5 lRXC<*?69+uJ>aQ&fSW~xI==&kzK^d-^5=gx6Y-&T?Hdo^jb8u& literal 0 HcmV?d00001 diff --git a/Tests/test_sgi_crash.py b/Tests/test_sgi_crash.py index 2b671244a80..6626f55f7c8 100644 --- a/Tests/test_sgi_crash.py +++ b/Tests/test_sgi_crash.py @@ -6,7 +6,13 @@ @pytest.mark.parametrize( "test_file", - ["Tests/images/sgi_overrun_expandrowF04.bin", "Tests/images/sgi_crash.bin"], + [ + "Tests/images/sgi_overrun_expandrowF04.bin", + "Tests/images/sgi_crash.bin", + "Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi", + "Tests/images/ossfuzz-5730089102868480.sgi", + + ], ) def test_crashes(test_file): with open(test_file, "rb") as f: diff --git a/src/libImaging/SgiRleDecode.c b/src/libImaging/SgiRleDecode.c index a03ecd456e8..46a9179234e 100644 --- a/src/libImaging/SgiRleDecode.c +++ b/src/libImaging/SgiRleDecode.c @@ -112,11 +112,27 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, int err = 0; int status; + /* size check */ + if (im->xsize > INT_MAX / im->bands || + im->ysize > INT_MAX / im->bands) { + return IMAGING_CODEC_MEMORY; + } + /* Get all data from File descriptor */ c = (SGISTATE*)state->context; _imaging_seek_pyFd(state->fd, 0L, SEEK_END); c->bufsize = _imaging_tell_pyFd(state->fd); c->bufsize -= SGI_HEADER_SIZE; + + c->tablen = im->bands * im->ysize; + /* below, we populate the starttab and lentab into the bufsize, + each with 4 bytes per element of tablen + Check here before we allocate any memory + */ + if (c->bufsize < 8*c->tablen) { + return IMAGING_CODEC_MEMORY; + } + ptr = malloc(sizeof(UINT8) * c->bufsize); if (!ptr) { return IMAGING_CODEC_MEMORY; @@ -134,18 +150,11 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, state->ystep = 1; } - if (im->xsize > INT_MAX / im->bands || - im->ysize > INT_MAX / im->bands) { - err = IMAGING_CODEC_MEMORY; - goto sgi_finish_decode; - } - /* Allocate memory for RLE tables and rows */ free(state->buffer); state->buffer = NULL; /* malloc overflow check above */ state->buffer = calloc(im->xsize * im->bands, sizeof(UINT8) * 2); - c->tablen = im->bands * im->ysize; c->starttab = calloc(c->tablen, sizeof(UINT32)); c->lengthtab = calloc(c->tablen, sizeof(UINT32)); if (!state->buffer || From 9a2c9f722f78773e608d44710873437baf3f17d1 Mon Sep 17 00:00:00 2001 From: Eric Soroos Date: Fri, 30 Oct 2020 09:57:23 +0000 Subject: [PATCH 2/3] Make the SGI code return -1 as an error flag, error in state --- src/libImaging/SgiRleDecode.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libImaging/SgiRleDecode.c b/src/libImaging/SgiRleDecode.c index 46a9179234e..9a8814b50ca 100644 --- a/src/libImaging/SgiRleDecode.c +++ b/src/libImaging/SgiRleDecode.c @@ -115,7 +115,8 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, /* size check */ if (im->xsize > INT_MAX / im->bands || im->ysize > INT_MAX / im->bands) { - return IMAGING_CODEC_MEMORY; + state->errcode = IMAGING_CODEC_MEMORY; + return -1; } /* Get all data from File descriptor */ @@ -130,12 +131,14 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, Check here before we allocate any memory */ if (c->bufsize < 8*c->tablen) { - return IMAGING_CODEC_MEMORY; + state->errcode = IMAGING_CODEC_OVERRUN; + return -1; } ptr = malloc(sizeof(UINT8) * c->bufsize); if (!ptr) { - return IMAGING_CODEC_MEMORY; + state->errcode = IMAGING_CODEC_MEMORY; + return -1; } _imaging_seek_pyFd(state->fd, SGI_HEADER_SIZE, SEEK_SET); _imaging_read_pyFd(state->fd, (char*)ptr, c->bufsize); @@ -185,7 +188,7 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, if (c->rleoffset + c->rlelength > c->bufsize) { state->errcode = IMAGING_CODEC_OVERRUN; - return -1; + goto sgi_finish_decode; } /* row decompression */ @@ -197,7 +200,7 @@ ImagingSgiRleDecode(Imaging im, ImagingCodecState state, } if (status == -1) { state->errcode = IMAGING_CODEC_OVERRUN; - return -1; + goto sgi_finish_decode; } else if (status == 1) { goto sgi_finish_decode; } @@ -218,7 +221,8 @@ sgi_finish_decode: ; free(c->lengthtab); free(ptr); if (err != 0){ - return err; + state->errcode=err; + return -1; } return state->count - c->bufsize; } From 1cbb12fb6e44da0d6d6d58254d0d96930d04af5e Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 2 Jan 2021 20:19:26 +1100 Subject: [PATCH 3/3] Lint fix --- Tests/test_sgi_crash.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/test_sgi_crash.py b/Tests/test_sgi_crash.py index 6626f55f7c8..ac304aab4d3 100644 --- a/Tests/test_sgi_crash.py +++ b/Tests/test_sgi_crash.py @@ -11,7 +11,6 @@ "Tests/images/sgi_crash.bin", "Tests/images/crash-6b7f2244da6d0ae297ee0754a424213444e92778.sgi", "Tests/images/ossfuzz-5730089102868480.sgi", - ], ) def test_crashes(test_file):