From 8dc4f659a13dbd707b0d44b602166a4413012d26 Mon Sep 17 00:00:00 2001 From: Processor228 Date: Sat, 11 Oct 2025 00:27:06 +0300 Subject: [PATCH 1/2] Fix cases when png reader ended up in an infinite loop. #774 Additionally, throw exceptions directly, instead of handling errors by longjumps. Longjumps lead to segfaults under compiler optimizations. Besides, setjmp setups always ended up throwing exceptions, so why not throwing them right away instead? --- .../gil/extension/io/png/detail/read.hpp | 12 ------ .../io/png/detail/reader_backend.hpp | 37 ++++++++++--------- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/include/boost/gil/extension/io/png/detail/read.hpp b/include/boost/gil/extension/io/png/detail/read.hpp index 089caf9fe6..e49da4b88e 100644 --- a/include/boost/gil/extension/io/png/detail/read.hpp +++ b/include/boost/gil/extension/io/png/detail/read.hpp @@ -84,12 +84,6 @@ class reader< Device template< typename View > void apply( const View& view ) { - // guard from errors in the following functions - if (setjmp( png_jmpbuf( this->get_struct() ))) - { - io_error("png is invalid"); - } - // The info structures are filled at this point. // Now it's time for some transformations. @@ -237,12 +231,6 @@ class reader< Device > void read_rows( const View& view ) { - // guard from errors in the following functions - if (setjmp( png_jmpbuf( this->get_struct() ))) - { - io_error("png is invalid"); - } - using row_buffer_helper_t = detail::row_buffer_helper_view; using it_t = typename row_buffer_helper_t::iterator_t; diff --git a/include/boost/gil/extension/io/png/detail/reader_backend.hpp b/include/boost/gil/extension/io/png/detail/reader_backend.hpp index 3f49070f3b..793a8ab1dc 100644 --- a/include/boost/gil/extension/io/png/detail/reader_backend.hpp +++ b/include/boost/gil/extension/io/png/detail/reader_backend.hpp @@ -87,7 +87,7 @@ struct reader_backend< Device // was compiled with a compatible version of the library. REQUIRED get()->_struct = png_create_read_struct( PNG_LIBPNG_VER_STRING , nullptr // user_error_ptr - , nullptr // user_error_fn + , this_t::error_fn // user_error_fn , nullptr // user_warning_fn ); @@ -118,20 +118,6 @@ struct reader_backend< Device io_error( "png_reader: fail to call png_create_info_struct()" ); } - // Set error handling if you are using the setjmp/longjmp method (this is - // the normal method of doing things with libpng). REQUIRED unless you - // set up your own error handlers in the png_create_read_struct() earlier. - if( setjmp( png_jmpbuf( get_struct() ))) - { - //free all of the memory associated with the png_ptr and info_ptr - png_destroy_read_struct( &get()->_struct - , &get()->_info - , nullptr - ); - - io_error( "png is invalid" ); - } - png_set_read_fn( get_struct() , static_cast< png_voidp >( &this->_io_dev ) , this_t::read_data @@ -625,13 +611,30 @@ struct reader_backend< Device protected: + static void error_fn( png_structrp _ + , png_const_charp error_message + ) + { + io_error(error_message); + } + static void read_data( png_structp png_ptr , png_bytep data , png_size_t length ) { - static_cast(png_get_io_ptr(png_ptr) )->read( data - , length ); + auto check = static_cast(png_get_io_ptr(png_ptr) )->read( data + , length ); + if (check != length) + { + if (!check) { + png_error( png_ptr, "read error" ); + } else { + png_warning(png_ptr, "read less than required"); + /* prevent infinite looping in libpng */ + memset(data + check, 0, length - check); + } + } } static void flush( png_structp png_ptr ) From 628165368ff2cd4c17d213aab9f0845b0dc9be73 Mon Sep 17 00:00:00 2001 From: Processor228 Date: Fri, 5 Dec 2025 18:59:42 +0300 Subject: [PATCH 2/2] Provide testcase that shows the fix in action --- .../png/EdgeCases/invalid-last-tEXt-length.png | Bin 0 -> 3041 bytes test/extension/io/png/png_read_test.cpp | 14 ++++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 test/extension/io/images/png/EdgeCases/invalid-last-tEXt-length.png diff --git a/test/extension/io/images/png/EdgeCases/invalid-last-tEXt-length.png b/test/extension/io/images/png/EdgeCases/invalid-last-tEXt-length.png new file mode 100644 index 0000000000000000000000000000000000000000..45df133ecdf0bdb4f9a118fc45d166c7c78ad48e GIT binary patch literal 3041 zcmZ{mc{J2*8^?b#_A!L)q^y}RW6$m}%AS3Dgu&RCp|KW&BD+w@7A9drMakY+$G#`a zJl1Ss5DE$Jbk2L;bKdiw_mAuTp8NXVpYy%{xX-z-L{pPn%#6H@001!S>mkf3YW=6^ zDGPvB*ML$)$W_nW7yu%~03iAy0Q{n~qSpZ+7zO|v&H$kD900h``Hg036yt)6;VlGk z{wRHbK-n>%_3Z8f0HpIzQ9Xr1_$W;}jJ~lh-8vI96aVGU7N|Z7)D~fBi!n!r3Za7n z+`W8JLYQzgN+`?^4FGzHPp!4t@?hZR%}bH7Qqyc|kD?obOS?4)P^z{P5#tUmHW9-n zsGSx8dxpDww49&f_H2UIFxTp!c!E(JRpqVXofTJAmEP@&>Nqt^p2v&TUeD}v!PDQT z6TpKNc{r`B@gM9t=IJG3b$x3L46_Cld)ywl6idG_k`q6F`(BZ~F2e)J#Z`QL>?Nl* z8{@{(wQH6mnQeX`3ibuPe8gM1peKv`=KX!qeRru8%Aab}EBh7(C6k`y6>0dUAgAGCt2; zr9f@!MOkrdg*5s>1R5mep9$$wdpeK}VtD=|4aJq8vioGS7>ClE++tBdp$)Yr0%s%;%oZ&G#v|kaWeV zP6t%L&ob07x!r2g7WHv25DFSP%C5!V7!Q`dA zX>XZ|g5`e3R%)VxhLVRVbv%v4-rAl<7hxf^|7MBdV%xsb>RLIvvq)}Way_LKQl!PFf(0+Y6k zL6OY$S(X!Z2Tft^esS8goRO5J+~>bspVJjinf7=cZMh5TJWAJAG_RnE<-4HKO`e>&OnC=8$9iJD(~#Tu1V zgk{?h31L%BEa*Iwd56oOmH@hdD_Zcch~4=H*Vty5Fb=6dnwebhGm2vr>;`{jUc&|$ z+4bdSFgQ-|YKXxsLf=sVBhGAIvB+qPxnNJ~)`|YUvZUyp+8gt-mZDP;a5Ye?r^^;6 zIIZ^Tb_V%bV_!oBx zbDhq@^pY?cp7?mh%b`9oiVmMY9WNgfz`XVwtIFvg95C0?lI150(}-CkW;0p;DF1Jw_V&1h zkZWE`m@uL+?8Pv+X3dex1xB0qw!QZDMS8~OGU}pZY5s5ZZ`LpUP~r0U^1_-Q4DI)v zsa(I}NK)GWl0fzMZ5O=pd*Vv3G%x0Wt6W`<1Ec+$IXJ^4fYGtXK=4FSa?0^uY5l{1 zZ6lfCJY)?@?c&jZrIP`xtRb@d~D-@|MLXDg#klB0jd{3Y@BM8Yxzk~p}e zjYyBqHeFhWYgRIRJs_fjTdU7V9d;XS>k*4PBSl+{si4f!d%?PXQc!ViG8`W_^qOzX2( z=arTioh3|BuaNU4&pE;eR2|c_Mx@mepN=&b#NKX>sIrY4!FD4apUXW7pK&^W$*hk} zryliNi9ZMOfr6Z3kjJ^z(h2NC2qr~P}fQc8F9_B=l^^C>L7%Yu|P_c?Kr zLkAh4D6*=97`$rQ62+Pc7Tw*Xu2GJpd4leDCMmJeJJ{myZLo|lEc9~gMG|gu%aV=|oSBbx9M<{h9S$Uqs$A8daLORHU89yYdkU*J~)0p$Y_{7jg%HAH>^%_yWxO%uJaBiRMo;WtiE3yK?h^|%B~`>lM>c9zBFY_4f?lv zFV4EFet&*vVr+TFumX84Fhn(MpVgz9G2!%%$ynLiyYr8!?`>%~z;#s#mD4fO`wQwZ z>dN-!%YiD!2DQVT48Q!c1lvlHoGygc2$6O^yFXKEW%!;WJz?w%dM zC10{r-EKRp4h=lkLB<`LXxW>bMmVx@aa=T@iq`66m6uvx6koI^g$qg)b-9Ez-Pp8P z>qDhX*%*@f_~t@4{(^On`aMeZ(P%>Ff2+?3{Za0A`oL+g3RG~QXHf%kp5k_@VibVE z7b@)Mr@oEzl5-AsuC|LPr)l;mg?LMrkAn~(02B+_A#Je`&9!5SZ-Y>`pq6adhJ1*b z9?vUtd0ozR>dg-J-83nOu`2q8RQq&C&S?d19&|E-v?F;{knu$4>u62xIp6in>TD8; zz5cy5V{~WfQM;ff>|_F7u{R?xrTNkm2%BEpZe%tfs&%F>IbT>^VZTy=H)#HXv8{fy zUBEa=&0p z;Ic&B$#DGv{o^8w2LjPS;L$;&#^AzyEyZ_%2P znVW>U{p@wqoq_~%m9N{(lR(Q&xvArKhquZ~dKt(VECEbe+( zBbuocSIQ77iqW;jxFInp71tmXMF3ftth^K)E(Mpfgu_*2l~v$yC=8|ogGpUGss1m( zFTl;qJ^cRz4j3I~3Lx>H1dNv->MjOJnP&edMDE`q`Uuop3d8n?LA$xDXdk!=nB4pO zR35taNyKhgo%4|5y8dykhMOaw{JHXmAGXaR9AAj^4?zk^n4|pNP(i4mYs2A@4=85= NeO(j8o4=f6{{`8LecAv3 literal 0 HcmV?d00001 diff --git a/test/extension/io/png/png_read_test.cpp b/test/extension/io/png/png_read_test.cpp index 0217aa9d67..3788268b02 100644 --- a/test/extension/io/png/png_read_test.cpp +++ b/test/extension/io/png/png_read_test.cpp @@ -690,6 +690,19 @@ void test_gamma() // G25N3P04 - paletted, file-gamma = 2.50 test_file("G25N3P04.PNG"); } + +void test_invalid_png_read() +{ + auto test_read_and_convert = [](const std::string &path) { + gil::rgb8_image_t image; + gil::read_and_convert_image(path, image, gil::png_tag{}); + }; + + // if any other edge cases found by fuzzing or by accident, + // you may add them here + const std::string edge_cases_path = png_base_in + "EdgeCases/"; + BOOST_TEST_THROWS (test_read_and_convert(edge_cases_path + "invalid-last-tEXt-length.png"), std::ios_base::failure); +} #endif // BOOST_GIL_IO_USE_PNG_TEST_SUITE_IMAGES void test_corrupted_png_read() @@ -755,6 +768,7 @@ int main() test_background(); test_transparency(); test_gamma(); + test_invalid_png_read(); #endif return boost::report_errors();