Fix cases when png reader ended up in an infinite loop. #774

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<ImagePixel>;
 
         using it_t = typename row_buffer_helper_t::iterator_t;

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<Device*>(png_get_io_ptr(png_ptr) )->read( data
-                                                            , length );
+        auto check = static_cast<Device*>(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 )

test/extension/io/png/png_read_test.cpp

@@ -690,6 +690,19 @@ void test_gamma()
     // G25N3P04 - paletted, file-gamma = 2.50
     test_file<gil::rgb8_image_t>("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();