From 2a3166e52310c886fe1f60b08bc1ceba3353532a Mon Sep 17 00:00:00 2001 From: Devashish Jaiswal Date: Thu, 10 Sep 2020 14:50:04 +0530 Subject: [PATCH] JPGLoader: Check existence of Huffman tables in scan header segment (#3442) DC and AC table IDs read in the scan header segment weren't validated against the IDs of Huffman tables read in the DHT segment. This caused an OOB read when a Huffman table was accessed using the ID read in the scan header segment. Furthermore, the decoder now replaces the old DC or AC table if a redefinition has been found prior to the scan header. Fixes #3439. --- Base/res/html/misc/jpg.html | 2 + .../misc/jpgsuite_files/offending-3439.jpg | Bin 0 -> 4437 bytes Libraries/LibGfx/JPGLoader.cpp | 60 ++++++++++++++++-- 3 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 Base/res/html/misc/jpgsuite_files/offending-3439.jpg diff --git a/Base/res/html/misc/jpg.html b/Base/res/html/misc/jpg.html index 11f199eabc..2f684fa721 100644 --- a/Base/res/html/misc/jpg.html +++ b/Base/res/html/misc/jpg.html @@ -6,6 +6,8 @@
+

Issue-3439

+ lena

Non-subsampled Lena


lena

Chroma Horizontally Halved Lena


diff --git a/Base/res/html/misc/jpgsuite_files/offending-3439.jpg b/Base/res/html/misc/jpgsuite_files/offending-3439.jpg new file mode 100644 index 0000000000000000000000000000000000000000..ad0d22c5fa1fcb60a7b9333f38a93d6e13ce8060 GIT binary patch literal 4437 zcmex=oIr{vTivovIz$!vMUve7&T5@$f4}C@t|nX z#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyoA)#U65s^{JDXD4c8JStd zC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$K>lK6U=!0ld(M2vX z6_bamA39wEztS{0OlC-n(x!Iuv_lBvBt*U587)s^BM5hvID$n%oh6S7^L$>W&z)T4KPx%T=^j`ipc@w;;A z&y}1jSNyF?^;*>$?p#SfoEsOZr@l4qM7a8gvsDM4R(@HxEBDilA6;jQ_&IVJ?7U7_0jS+WVZaCt@^d8>UsaHv)eMA#HSv;aYU&4*T-2MPB))4SkLLx3Nk!x@vlTB zE>FPXXMffT$L$;My>JS9wo7?}p4-93uS-M9B+s$uTx)*$(KRP7$z`sCZ2SJb>8Gt0 zi=5E0-LdfCTEFs>Vo%SeD8F;Nb^qV4S(Z#H$$8=Pr_9^)yx;%dR;>=H<3{uE8Q0AA zS17Tc|7FE8Rrz#=T4?FL8D{P)Z&VxzW zxes=F+;ZJosmwfKZ#`plmFnE3yV$0){Ju7A;mTW%dH3S(GzRug4*X{Fu=LJ#-msA8 zZ-uXXwoGYC`sa{)*URWZ*SV>FmuFqyeI;qyhYbgg%s#2c?Pxu@z|ey59CEh{?|*!v`6+S$iN;(G1x%DT|0Jgt(8A>|Ip(7 zOZC4o&N8?zWRl;<^i@78#Zh%)T;#C}MMZwT(XoEcd8fj8FZyz>y%DltdCFGJwR^-m zC1&1y=GVb+@cGUaPy1P;e2fJzNXchD-4yeAmO**S=YXe0p>olRiEZ{41`gKK&Sxbg zT@thPwLE{Kav2}T%E?wjU6Y=M$z=N2V{rSg{%htVEuxzRBfrmj#oZoHg zdqeFzj!Vm>tjT^AE^m0;cJ-92%h)n>H}}ka|JQzP?wxZgMMoaGtIYR$ygo%-+_!99 zk-xm()jJa(KNsEGCm9rd`jd6G*lnf-2nLRN(3NF_$jh_+hDS z?3;4Ta;4hNibuL4`=XQAWvfL`+M}CtPr7^At(1g)$EN6?Ep*LtEYQFA@W!k5$@8{^ zF#G!yaW2|zx8ZDF!tL0SlWXr*tzzb9&$Q?ygBWFLrP3YmCd@`DZJ8 z!n&rjc4wM5=I70DxIJt2wuM~BC%9$HefqkNGxYb?)16101UKEeB$%_}O_bB&YrAB% z0vaRzE;+o&o)@Ze^>p_9%}o0%8z1mB?W^0eYxCVx!BgF(GyXF$=f_RDx#os|80!Xx zxX1Q?vz|>6DAKA_$&}eWXXC3Z!x*EfH@}>kt~)tN?6JaM{-u^zx2=}zz0lPftvdPA znwXV+8XHgLN+{o7|Jt?l`L6thVs)|0pUJf?{~Y5K=y|(u=CcJ{X9~AGe;K?;)H3B6mM|z)Mk2||0sVTa7C(alk^Tvmwj6IpTRL?OO))X zUCT2T@#M&Sst&LUJZbaUPlhXcvZm$fnLQH{vuv-bEPrBiRas9fCa*Yr%|fLMHGNxG zGyFE~>VG`teg5*qf3?|C2YIi4aK zx5tV}e#?xb$BfRu&|X#SWfnbObjIU2wbiqK>MQ2iSj+V;JL23n`^Q;lrd0E^6|dt| z`UCg%NvEla+HCx%9Im9|q&;g<)^(ei_sg^!BD&hsii4HP^S)X+c=u`Pr}(~c*>-wM zp=)Mkmc@xbFIMi9IWGJy^~-V{hLA5i)_Z(8wnC$bkJt9;#Os;GU50L&ch?FtwY*iy zoijnE@-aQ%t9&e}$p3NVt|)^UF)~wo zp76gnPmHz6n7*wpd6UlS!#)4Do{o%tQEL`(d+B@avb3}!O)Y1(YswV|-m%&^EdLiJ ze`;rCjp-R8u1hb`=lo2$pK8Y2Op{>B#!D=%3*WKjD2s?s!9B51j( zdTqzKr>Ct8YW=j-=Z2ay@h@1Bci(8voW`Sp`A?mfoy)SgDj7RFV%bIxONCI@{W;0XA6KZ(U1FI$YbtxK->Dft zqCV=z#5S2N{55N_%2a93vu=Km7~V&%kn_#)S@!O1u3mVTrlcQFdtUX#@PJ?cO1(sH zx4u|q7{+t)(7co-Z&}i1vm+KyTk2mPzDU0Ck+iPd#f6u2kKc-^`;_)3d)Cf%$^j?QoW9%WA%-pOIQN zcjb?|^v0e}!JLm5u8LmMTV(Sl@!gJppYae`B2y58>LeUR~1O#9>nlO;v|SueK! zXJEXYr;)ZnJGZ!h_aDC6zxG^RQ>>1B+fcbFtT%tpr=CZ%{7tiiFTeY;dfE0pjfZek-?7T_@vNsA-xH&Z!@D)_yC3^_d~N!?tt%_sr#D}@7G1hlv05(9P&Md9rpc7H zCWo~`7PkfF1c`|pOUm52YUAV?YpzV(rs}_9LDOM|T8*}Q?lWYUXU&iJ#Z{(y;$95n z-!o$pG}W_m8%w{v#R8Lapcp_SDsw!l0C=cT326YEq*^FbI#A12Unfj zU3B`w)gs50$`cs3?g}khy7Bn713kt^%V){V%Uv|7f@uf4vvgkk?$~LQ3#RQ7D5@)y z;X1Xaa$3ukAKur)?MjX?e_GEpBmU{pbk3jW*G6QP9f_IOwDYG=PQa}A`}Vdk*3~@@ zR(p8j>fVU|1{>i)>(>`l4n zOKwC*-gJL*tYPX&h0yMmXCkj1gP> zVR3Bh$*tAVQ{Qg=HvhzbhTh^;MQy!uy*{~CyWX6)RW>;md}-^}x!i#l)ICb2>s9aM z1@4Vj{$M(@%`{44*Xo^|BCLP*z6#nQyYliu&Q0!__7xF-nlH`l`6w98bjYdgIQ!Bp zot2r3Voo^RdA+f2dDn?;m0<}55$3G5S@+gY@3TmHrt$Mx>(0|wn`T*lo|Udb z#^&a*Qp-PUO(s+-ZEWCSnJ6PQ?N!;VE82WhC)Mv1bCW#hFB0<9ttv29=IGot&L*E0 zNNKDqiD8?!V{Jd5*YKSSbL?k@?J?X~*NUp0TWnW}oOPvTzlL5$EEVCp@8XQv#!pC(PLKY|i3Y8mkn-LY1eA zsC#MxZRWn?UzrxdLDIYTl~>Dn^XMefZ^k>i@LOn Iga6+I04E4Mxc~qF literal 0 HcmV?d00001 diff --git a/Libraries/LibGfx/JPGLoader.cpp b/Libraries/LibGfx/JPGLoader.cpp index bec4d6f5b8..e714ba12c6 100644 --- a/Libraries/LibGfx/JPGLoader.cpp +++ b/Libraries/LibGfx/JPGLoader.cpp @@ -536,8 +536,31 @@ static bool read_start_of_scan(BufferStream& stream, JPGLoadingContext& context) stream >> table_ids; if (stream.handle_read_failure()) return false; + component->dc_destination_id = table_ids >> 4; component->ac_destination_id = table_ids & 0x0F; + + if (context.dc_tables.size() != context.ac_tables.size()) { + dbg() << stream.offset() << ": DC & AC table count mismatch!"; + return false; + } + + bool dc_table_exists = false; + bool ac_table_exists = false; + for (unsigned t = 0; t < context.dc_tables.size(); t++) { + dc_table_exists = dc_table_exists || (context.dc_tables[t].destination_id == component->dc_destination_id); + ac_table_exists = ac_table_exists || (context.ac_tables[t].destination_id == component->ac_destination_id); + } + + if (!dc_table_exists) { + dbg() << stream.offset() << ": Invalid DC huffman table destination id: " << component->dc_destination_id; + return false; + } + + if (!ac_table_exists) { + dbg() << stream.offset() << ": Invalid AC huffman table destination id: " << component->ac_destination_id; + return false; + } } u8 spectral_selection_start; @@ -576,6 +599,34 @@ static bool read_reset_marker(BufferStream& stream, JPGLoadingContext& context) return true; } +static bool huffman_table_reset_helper(HuffmanTableSpec& src, JPGLoadingContext& context) +{ + auto& table = src.type == 0 ? context.dc_tables : context.ac_tables; + if (src.destination_id == 0) { + if (table.is_empty()) + table.append(move(src)); + else + table[0] = move(src); + return true; + } + + if (src.destination_id == 1) { + if (table.size() < 1) { + String table_str = src.type == 0 ? "DC" : "AC"; + dbg() << table_str << "[1] showed up before " << table_str << "[0]!"; + return false; + } + if (table.size() == 1) + table.append(move(src)); + else + table[1] = move(src); + return true; + } + + dbg() << "Unsupported huffman table destination id: " << src.destination_id; + return false; +} + static bool read_huffman_table(BufferStream& stream, JPGLoadingContext& context) { i32 bytes_to_read = read_be_word(stream); @@ -594,7 +645,7 @@ static bool read_huffman_table(BufferStream& stream, JPGLoadingContext& context) dbg() << stream.offset() << String::format(": Unrecognized huffman table: %i!", table_type); return false; } - if (table_destination_id > 3) { + if (table_destination_id > 1) { dbg() << stream.offset() << String::format(": Invalid huffman table destination id: %i!", table_destination_id); return false; @@ -625,13 +676,12 @@ static bool read_huffman_table(BufferStream& stream, JPGLoadingContext& context) if (stream.handle_read_failure()) return false; - if (table_type == 0) - context.dc_tables.append(move(table)); - else - context.ac_tables.append(move(table)); + if (!huffman_table_reset_helper(table, context)) + return false; bytes_to_read -= 1 + 16 + total_codes; } + if (bytes_to_read != 0) { dbg() << stream.offset() << ": Extra bytes detected in huffman header!"; return false;