From 3bf5eddb89afdf690eceaa52bc4d3546ba9a5f87 Mon Sep 17 00:00:00 2001
From: Eric Soroos <eric-github@soroos.net>
Date: Sun, 7 Mar 2021 12:32:12 +0100
Subject: [PATCH] Fix OOB Read in Jpeg2KDecode CVE-2021-25287,CVE-2021-25288

* For J2k images with multiple bands, it's legal in to have different
  widths for each band, e.g. 1 byte for L, 4 bytes for A
* This dates to Pillow 2.4.0
---
 ...b027452e6988530aa5dabee76eecacb3b79f8a.j2k | Bin 0 -> 335 bytes
 ...4c83eb92150fb8f1653a697703ae06ae7c4998.j2k | Bin 0 -> 335 bytes
 ...ca68ff40171fdae983d924e127a721cab2bd50.j2k | Bin 0 -> 335 bytes
 ...c93af851d3ab9a19e34503626368b2ecde9c03.j2k | Bin 0 -> 3886 bytes
 Tests/test_file_jpeg2k.py                     |  16 +++++++++
 src/libImaging/Jpeg2KDecode.c                 |  33 +++++++++++++-----
 6 files changed, 41 insertions(+), 8 deletions(-)
 create mode 100644 Tests/images/crash-4fb027452e6988530aa5dabee76eecacb3b79f8a.j2k
 create mode 100644 Tests/images/crash-7d4c83eb92150fb8f1653a697703ae06ae7c4998.j2k
 create mode 100644 Tests/images/crash-ccca68ff40171fdae983d924e127a721cab2bd50.j2k
 create mode 100644 Tests/images/crash-d2c93af851d3ab9a19e34503626368b2ecde9c03.j2k

diff --git a/Tests/images/crash-4fb027452e6988530aa5dabee76eecacb3b79f8a.j2k b/Tests/images/crash-4fb027452e6988530aa5dabee76eecacb3b79f8a.j2k
new file mode 100644
index 0000000000000000000000000000000000000000..c9bd7fc0a8dfe76af64b373351349c55a1e5de18
GIT binary patch
literal 335
zcmZQzVBpCLP*C9IYUg5LU=T?wsVvAUFj4@r8KAT?kj?;d#WFKeih#5N7&Ec6GXfb5
z{K@$_MPL?#AdoAToRXTxzyy*30!APN1E3Mf|NZ|5GU$N03P2KsVL$=M0AUDVVsG3r
zgOTxn5CacTfRTZfg@u9fe++}Lf=56=1Plducm(`U0b0uFotT}NQmR{Ks%NBU_<sTe
z$Ri94H9$R+k6&S7oB<>_0+v5uVf%dk00)ch;<qdeJmM#tkKbZr;AIkA_?4-F;r!KD
zgTt@b7?h+qlB}y1{5;ZNm$YbYNAAXl=?6b|Gng%$xh{FJX~yd#mlphFVVFB@B_oTr
Zz>R~Sco^cBAMaSeyCLBPTkYfjHv!}sSR()c

literal 0
HcmV?d00001

diff --git a/Tests/images/crash-7d4c83eb92150fb8f1653a697703ae06ae7c4998.j2k b/Tests/images/crash-7d4c83eb92150fb8f1653a697703ae06ae7c4998.j2k
new file mode 100644
index 0000000000000000000000000000000000000000..fd2f4dd367773b472569bd41715b6892aa4f74b3
GIT binary patch
literal 335
zcmZQzVBpCLP*C9IYUg5LU=T?wsVvAUFj4@r8KAT?kj?;d#WFKeih#5N7&Ec6GXfb5
z{K@$_MPL?#AdoAToRXTxzyy*30!APN1E3Mf|NZ|5GU$N03P2KsVL$=M0AUDV5^rQ=
zXJq^z#J~d-U}RurVPRnWAHyK5;1LiI0Yd>E9s&PTfR-|PCuS$6l<Jn5>KW-7{-3}A
z@(2S%4N%YI<5yT1X8;L~faMQZ*gl^>z`<g>_$><qkNC;v<G0utc$owjer0N4IDa+P
z;P5Lp1|=zuq<>WlejaJCOIozHBX{G&^n;(f8O#>WT$jAqG~@M=OACIoFwC8{l95GQ
Z;KspEJPh&6k9REK-H`Btt@iQ%n*cICSFQj6

literal 0
HcmV?d00001

diff --git a/Tests/images/crash-ccca68ff40171fdae983d924e127a721cab2bd50.j2k b/Tests/images/crash-ccca68ff40171fdae983d924e127a721cab2bd50.j2k
new file mode 100644
index 0000000000000000000000000000000000000000..c3ad0d6330a7920f58a809e01c8691014ad70fe0
GIT binary patch
literal 335
zcmZQzVBpCLP*C9IYUg5LU=T?wsVvAUFj4@r8KAT?kj?;d#WFKeih#5N7&Ec6GXfb5
z{K@$_MPL?#AdoAToRXTxzyy*30!APN1E3Mf|NZ|5GU$N03P2KsVL$=M0AUDVl8}&K
zXJq^z#J~d-U}RurVPRnWAHyK5;1LiI0Yd>E9s&PTfR-|PCuS$6l<Jn5>KW-7{-3}A
z@(2S%4N%YI<5yT1X8;L~faMQZ*gl^>z`<g>_$><qkNC;v<G0utc$owjer0N4IDa+P
z;P5Lp1|=zuq<>WlejaJCOIozHBX{G&^n;(f8O#>WT$jAqG~@M=OACIoFwC8{l95GQ
Z;KspEJPh&6k9REK-H`Btt@iQ%n*jHaS6u)A

literal 0
HcmV?d00001

diff --git a/Tests/images/crash-d2c93af851d3ab9a19e34503626368b2ecde9c03.j2k b/Tests/images/crash-d2c93af851d3ab9a19e34503626368b2ecde9c03.j2k
new file mode 100644
index 0000000000000000000000000000000000000000..3aadfc3772717bef34360b9138f56365a07728da
GIT binary patch
literal 3886
zcmV+}57F@dPybN>DF6Tf002M$002M$0000000000002M$002M$00000000000S^H|
z|55-90000100jgD00IA8024rfh=`Dgh>(bgkcfzoh=`E?WB?@q0Yh?SVRU6=AYyqS
zPjF>!N>D{dAa-SPb7^mGATlm6E-?R)015yA000iP00IA#&-^GL2!eVy;#fdGzys+E
z01u=r03S#|4l;{9uLsQxrJoQy_`|>h=^X$Mq<jEA!2lWFn1HZ6T^cnn*5`>|6fL4Y
z_?hYHHB+s)I`GjNhH%}k`qLl@|7B(lV8$=N2iR)>A7RV@KEt3#hu`C4V&TdK18uF9
z(d-MFdc{vNj<9B$$}zN1!zDjVfH#?!L6Qk)gd05o?a2zrN;7YtLhT8nTx>?l-jo^x
zo(`~QndSf{1$dXB$~9L%!nbA5Jct1r3jb$yADnLO05Xz;zEUGPiyGNQt3=Sb5~1T=
zzUF}wE7Q}?b8pf-3aqJuA+uPICxoz2RKa71l6sz&V0PxK;Njz~G>0@Rs=O=*lEP2G
z2ia~w55DDq2ibUlqSk+m1dPG!Y&6OzWi#35Q)*|ygwA68jBd<iiK(AK9(^NazoPw6
z3$V^%&ng=26KQLUrzrX`DUSso_QHT=MKNFXQssOFe^h$pC_vw8AfepD^N1RQ25iY%
zj$T__t7yKA4g!XY?mMso!SAV|7wW9{>54ysCK`?+1WLKhD*Xq49=T1FwatpUH9Q!d
zzktA1+K0oUeO^2*I2v_D%B>+D;BN#By&Gu8J2n@GfC_9E2pZN*VUi@*G#thOX#t#b
z#3_x9;eNrWyJYOP0I+$waO(=%z+jbsp1_Oa>sC-SrFoL}(adZ@IY>igj{jx@hIc}&
z#YVSccfAd7>FZ?5>0ZUT?xV7oricoxw{X8@C&(|RV`uF3)e*3n&3?yquW?JgZg&OG
zEl+FSunjB42_1;8%L2Qwa>-{+($T_mud7-b6f2pU9S}~>vbIgY9z^~b$}m}#0mG;u
z4JFaIXPM)o%);`>%o=8)a4w(IPs%Re5<>O9XZ?nojtGcw3{2;(OE6Fc{JppMTrlM~
z0!#J@O*i;x^|{-XJRrFkffj_Gw^^t7YIe9Frj~A^{7!YYF$D%>!X~k;R~j}bVvVh^
zDdBa?uLS8(`R1%Ty%&WugcxsV<sHY{xz*t5pG8gxDO<FHY3L}F`W+t@>shOtH_);)
z-J1zue9b9YZ2Q*{LPH%ya3oA*>yXnzU|mm3-<pZ+;vVUSR}lA2K}YrD3^n<`6?SEh
zgv^H~X1rUANX9_qTXK7j|6~Lt2ANByBw0NMwFyR*FzWr}<iY_MK;qj5Cz6<i_0ev$
zX)CZ2q@><xVc+ulq-`I#BlM}Ixs=CJIQOR&XfycQ5!lJ!+uSD*@I_E9fKsIh96U&U
zhfS<)hmKlRB#KEjpUjt|(3(;Dw~9)s&nCbJ+1u0reVxld1MKeHS_^kgb}#Q8a^E7U
zv2hX_xO#;%ZbXJRLA0&-Gp<*I)>w1(Hjgc&34#uTi9z$Ft9_h`tc(PB-#5+r^OI=!
zM95egr8YrS1ZLTTnrLV?CF9^kYmp7>Va`NNR&=<EeMqt?jB$QiG9YT>__<7%*Q&|t
zS6H$rkR_xE$L0vdc&>YOjH&Z!`Fe*$5F}x?O`lp`4z+xpmzsIyhaWN17qDZl*rg`1
z1Y}24|0hGz;(5dS{T!x8+D*~7YDJyavcow}pB#2S&D6=P+~-)@PoZ^-M4#gBNkqm>
zI`w>bL5MJYx{IYp+UF^-m8X-Q4B#ucG&Q3TN{0n<f9pnsS!?sw%H08~Wl}R6honcT
zD=yAHrtmT=XWc<2Of18Z3^E1He>>%haL9;#Zw9%PFDsK`Qvq`8S2SKJ)tOewcqVfl
zdVw@Awn<nYNKFi@>KVjwcDSvXk)o4Yc?Zz8GopOl`V(Ls^zE0l=W%kv3576kXGv)c
z9X9-j(jF`Mv1CjWw$FA~KdxLMPpb$m=<p<jm`wnhBhJug>nUE^i{N(1z=1IO+6fl4
zz)|!~*s{#IzX0e9Xf94+Emk72+aR(EW>!0YKm{0v-x;QEY&)W}31L9xH9qOlElBV(
z_0;|bk7~g%SlSPQ#h<XT*ob8E)ewt?j5!|F{7%9z<w5fBrK^awlB8*0b0Z@}Xf1%n
z(`7B&I@71)A<c73X?gZ9O-~eD?W2}m{~XBUgR*_0LmWx;Wow4>QnRf2SyRW);Aw=v
z3eJ|qzt=zxgG`l#Y0(giqTh7du7?V~#8QnjN(_V9_c!bOHxy){Z|NvPG|e2`_%ro(
zMiGL4=7l>>N&sc)<T1PKikL5vK+#oC=vFvWY*3Jb-#YE&{i4P=xI#SxM=Un!tWPZs
z&fMjNSX{z$5XsSCC?$cpy<8!q+Ng{EOJf06^%&&vYk_#7@p13rYA^t`HS9|~Uu4|A
zjs>plOU|W9%m;dq{Fuv(2<AKj8Q=dgLX*$U<)puSRhI@tPVb2lRV#$i-2sFdz5gkk
zg$p|aiq&JBGFuq+4of-`_4qi4&rbGotLDsT>rIq&I&qZ2)^`0;csbi~zL98YR)x}x
z25+gNOQ7WNwg^sg!SiDR#%dT?L?4_&2JA@oB~S0WD7lNSZxr1~VY)}-SG_$(>VlZS
zE9@vqRLQaZV=1c-ZhK>Fy?jA^FVk5HeTqEN6+`_}aq6%V`dDK+8zK@A`Ry>+tV1i%
zkx3LyjSy=V0(R7#z(0dYYW10T^LiaxZ%)mKm^f;&ax5{0MJB%w;&=-~j``0Hc}1V{
z*PhNl6bv&Y<o%=As6@<tju+m?DVyk<t65Dt8RSH@bc%&p6{q3XO+z`j30HXvxQ_sA
z#L60-%}QNl;%^7Z^TRFl*RNF3QW?YPW5ydu2AhH9&5VX+<PfDB-_44IS8k;>8YB%%
zTu7s#t8f#0WpwdzD{fvIzUwY(Uu5x_==rXQ%ljkjsH<xDI5M!&h?h%8k09_{!QELk
zDxTFX*CdTw-baV*m|Xu)VH3<krjl}5H<2HP_}lj2f3b4W%l~2!tubF@B{%cX){It_
zl1j_g@LeI<&@l$Vegces=yko3JMRNzwIbBFL;7UyWHJqh;U**D1H7tpX_+cj;5|9p
z@I#wvin|zANbhO>)Hv@81IHV@6o$WS#Fx}Q=^EDS&ZDGOg=Oxf`9Kh-AdVKOt#<On
z%<JQ~6<0Ayh@9_lqM!eO-;rH{jN-Z>I99m>Qkr%Fl0qqG>!-|T{qFsmOKma(bnm@0
zs>HC2-MI;6DVjUG$P7MdCA1)`D|$R4JKmh_LDd8<SiQ>GUrQLI7P%u$<HabJi+JJP
zk()PQrVgw_K!5)`5?ElprYcpCt+}dhpJ4XFwVpjr<#yS*$$3M~b3E(6VBTBxC?D5^
zQgc|;GggnTjZ%N0GiM*3?^muYQfk2JfCJc<9Z4!5=$fgQ_fb>Hs-G+7u!WR;!}Gmi
z@>oE8rFW#av0rY0#9lOsd%X0M-dU)(cIBZb?3vDp4XZ#8S$JU^YfqY%`Cs#?^fy@t
z&7ceN-z-}(j&#Gp6hX!kdsZKV=vhQwnb(b!HNQ_jfK)~6{p-+~GuA+^nVbC=Hu$rB
zpw^<@^6|)?2IvU4;S-&LJ4?q$D3YJp(t}}BtIkA9JDu$dHUrsq)>e6d%O3hcDkK)7
zeY^wh1?Z<a3FVFjkCwH!b!BKj57Ul<Y<+>n5gnfhh!#2}GDpE82$L#s0bYZg9qb=9
zoHqx`<R##}cgX*GgC<@om$5XyHl_hr)?Tqqejr7h6A417w+p|eS6&AW!Zu;vD}o41
zM~HEAWWs`gxYi3#>HI@(rA`kAHLEz*dL`TkF~uiY$P#9;kBK)1!t#LqwDl^u5{WQ?
zY>ekVp5S>y((<P78!eR<Vtqm-IO;U}ifPc0r4+sVd71-IU*Ga;XcnZASzXeGh@Zys
zJNyW=3C2B|1gSpaBZC)=h<Z&zI4aa7@*1vkCEB{kvbtRtA&kYZ+))n%EJR&j|7uBs
z2)Ru`SgE97lRZG<!3>nmNQnFAjUnjA0wyj$H``b!#p-84?DJP&7em>gAkC^Tx`)LD
zQ*se~7A4GEl0}mjIlQlOYfmpz3IiK|Z9N5C;0qR>gPp~)bLMp{Qiw{T$vu>lB;q&g
zaO|eZj(_tlWJ%Rx$-_CVk-pWSaxl0f5#;2@AoMFuZ`OWK92i|rC#1S=k2ZL`ZdIym
z>`6Y^%QTVnf)?19ay`myg$y*Cpt2UPsw}d_duathy&B1gLmH3Q`IqB%dL|(F-?@M{
z3@8fH#pv$r2kA)z2?%Rfj(zuEowCN}DJ*9eE86CO$$xjkD%2XR@0dz0s!nw8ov2Qo
z73uiy0qVCWi`n!5twnxX$5ZY+hf<0jj73LFv%{(0u*V##M_%_VI&_%HsW#R1<MX~V
zl}Y+6jf<suj84%A@<?k{8hgyo(_A~^Y&N*4z{|kiO#n$;XEGt1&v^TxeIXx3G)7Q?
zdbfS6@yU{dH56&%R0nHV0Xni}X3K)XCErA&jhCcf<>FlnCT}i3d&*9lJzqMu#gt&A
z)^jrCipO2W{9yJ*=Of_<T`B|FS40I#0-aMkP;JwY8@o;?%#nRjMPSZ(N|uvL&g-*Z
z2LvSG-`BXJIMac%)7jlZe&mE^v=U*;R4?eD)K!1?Y&UyJ(k4hiJb#e+=L>}9iK^t|
zVFM!$r(+@ti0&<3!zrh>%+8;yAFkH2pRd601PMhWW2q4u&Rj<ZE45U4UtbeNz~xPN
z0xhtOwb5&vUYZ&$Cr(5~i3uBEYyUF&o&~+B_O*+A#+}TpTi|QmBk2)8K)B8?%%HNf
zKXO)O=EPFYvA)b$@44mirEg~w6yX^Rc2l{*O5*YODnaTEqgL+~npx`Uo91%)X##m)
z{$_v4S!W~VwL5<fw-o+MoIjUbL(d|tJxF<P8Ra7e#JPv7<_D)#H>s=F-*%SX*W3sj
zQsmE@+6&m8iKrop4=zFmY^#3`RS*|^?N}(WYj7eU7?s-AnPN`RTd~zkOe^nuHY9;{
wPDcwS(s{EPCAJ{f#}b^oPT0b|?CjRuDmg`9He^Cv`gDOO^8O3fGe3X-*^~QNr~m)}

literal 0
HcmV?d00001

diff --git a/Tests/test_file_jpeg2k.py b/Tests/test_file_jpeg2k.py
index 13ae09af59f..5523d068b2e 100644
--- a/Tests/test_file_jpeg2k.py
+++ b/Tests/test_file_jpeg2k.py
@@ -231,3 +231,19 @@ def test_parser_feed():
 
     # Assert
     assert p.image.size == (640, 480)
+
+
+@pytest.mark.parametrize(
+    "test_file",
+    [
+        "Tests/images/crash-4fb027452e6988530aa5dabee76eecacb3b79f8a.j2k",
+        "Tests/images/crash-7d4c83eb92150fb8f1653a697703ae06ae7c4998.j2k",
+        "Tests/images/crash-ccca68ff40171fdae983d924e127a721cab2bd50.j2k",
+        "Tests/images/crash-d2c93af851d3ab9a19e34503626368b2ecde9c03.j2k",
+    ],
+)
+def test_crashes(test_file):
+    with open(test_file, "rb") as f:
+        with Image.open(f) as im:
+            # Valgrind should not complain here
+            im.load()
diff --git a/src/libImaging/Jpeg2KDecode.c b/src/libImaging/Jpeg2KDecode.c
index a2a7354dbe0..aa4fc58f7a0 100644
--- a/src/libImaging/Jpeg2KDecode.c
+++ b/src/libImaging/Jpeg2KDecode.c
@@ -644,7 +644,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) {
     j2k_unpacker_t unpack = NULL;
     size_t buffer_size = 0, tile_bytes = 0;
     unsigned n, tile_height, tile_width;
-    int components;
+    int total_component_width = 0;
 
     stream = opj_stream_create(BUFFER_SIZE, OPJ_TRUE);
 
@@ -814,23 +814,40 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) {
             goto quick_exit;
         }
 
+        if (tile_info.nb_comps != image->numcomps) {
+            state->errcode = IMAGING_CODEC_BROKEN;
+            state->state = J2K_STATE_FAILED;
+            goto quick_exit;
+        }
+
         /* Sometimes the tile_info.datasize we get back from openjpeg
-           is less than numcomps*w*h, and we overflow in the
+           is less than sum(comp_bytes)*w*h, and we overflow in the
            shuffle stage */
 
         tile_width = tile_info.x1 - tile_info.x0;
         tile_height = tile_info.y1 - tile_info.y0;
-        components = tile_info.nb_comps == 3 ? 4 : tile_info.nb_comps;
-        if ((tile_width > UINT_MAX / components) ||
-            (tile_height > UINT_MAX / components) ||
-            (tile_width > UINT_MAX / (tile_height * components)) ||
-            (tile_height > UINT_MAX / (tile_width * components))) {
+
+        /* Total component width = sum (component_width) e.g, it's
+         legal for an la file to have a 1 byte width for l, and 4 for
+         a. and then a malicious file could have a smaller tile_bytes
+        */
+
+        for (n=0; n < tile_info.nb_comps; n++) {
+            // see csize /acsize calcs
+            int csize = (image->comps[n].prec + 7) >> 3;
+            csize = (csize == 3) ? 4 : csize;
+            total_component_width += csize;
+        }
+        if ((tile_width > UINT_MAX / total_component_width) ||
+            (tile_height > UINT_MAX / total_component_width) ||
+            (tile_width > UINT_MAX / (tile_height * total_component_width)) ||
+            (tile_height > UINT_MAX / (tile_width * total_component_width))) {
             state->errcode = IMAGING_CODEC_BROKEN;
             state->state = J2K_STATE_FAILED;
             goto quick_exit;
         }
 
-        tile_bytes = tile_width * tile_height * components;
+        tile_bytes = tile_width * tile_height * total_component_width;
 
         if (tile_bytes > tile_info.data_size) {
             tile_info.data_size = tile_bytes;