CVE-2021-39432: Double Free (CWE-415)
value: diplib v3.0.0 is vulnerable to Double Free.
CWE description: The product calls free() twice on the same memory address, potentially leading to modification of unexpected memory locations.
CVSS v3 base score: 6.5
Published: 2022-11-04T17:15Z
Repository: https://github.com/DIPlib/diplib
Language: C++
Files changed: CMakeLists.txt, changelogs/diplib_3.1.0.md, src/file_io/jpeg.cpp
Commit date: 2021-08-12 10:37:57 -0600
Commit message: Fixed error handling in `dip::ImageReadJPEG()`, `dip::ImageWriteJPEG()`, etc. Fixes #80.

Diff: diff --git a/CMakeLists.txt b/CMakeLists.txt
index 96546898..eddf21e4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -41,21 +41,24 @@ set(DIP_DESTINATIONS
 # Compiler flags
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # also matchs "AppleClang"
    # Compiler flags for Clang C++
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wconversion -Wsign-conversion -pedantic -Wno-c++17-extensions")
    #set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -march=native") # This is optimal for local usage.
+   set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
    set(CMAKE_CXX_FLAGS_SANITIZE "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
    # Compiler flags for GNU C++
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wconversion -Wsign-conversion -pedantic")
    # "#pragma omp" causes a warning if OpenMP is not enabled.
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-pragmas")
    # "enum class DIP_EXPORT" causes a warning in GCC 5.4, fixed in 6.0.
    # "DIP_EXPORT" in forward class declaration sometimes causes a warning in GCC 6.0 and 7.0.
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-attributes")
    #set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -march=native") # This is optimal for local usage; to see which flags are enabled: gcc -march=native -Q --help=target
+   set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
+   set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
    set(CMAKE_CXX_FLAGS_SANITIZE "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Intel")
    # Compiler flags for Intel C++
    # TODO: compiler flags for Intel compiler
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
diff --git a/changelogs/diplib_3.1.0.md b/changelogs/diplib_3.1.0.md
index b39bc8d6..30892d85 100644
--- a/changelogs/diplib_3.1.0.md
+++ b/changelogs/diplib_3.1.0.md
@@ -100,10 +100,12 @@ title: "Changes DIPlib 3.1.0"
 
 - `dip::MeanAbs()` and `dip::SumAbs()` could produce wrong results for complex inputs.
 
 - libics had a typo that caused out-of-bounds read (#81). 
 
+- Fixed error handling in `dip::ImageReadJPEG()` and `dip::ImageWriteJPEG()`, which previously would crash when libjpeg produced an error (#80).
+
 
 
 
 ## Changes to *DIPimage*
 
diff --git a/src/file_io/jpeg.cpp b/src/file_io/jpeg.cpp
index 1663be4c..6f8b10ef 100644
--- a/src/file_io/jpeg.cpp
+++ b/src/file_io/jpeg.cpp
@@ -22,35 +22,44 @@
 
 #include "diplib.h"
 #include "diplib/file_io.h"
 
 #include "jpeglib.h"
-#include <setjmp.h>
-
-namespace dip {
-
-namespace {
+#include <csetjmp>
 
 // JPEG error handling stuff - modified from example.c in libjpeg source
+extern "C" {
+   static void my_error_exit( j_common_ptr cinfo );
+   static void my_output_message( j_common_ptr );
+}
+
 struct my_error_mgr {
-   struct jpeg_error_mgr pub;   // "public" fields
-   jmp_buf setjmp_buffer;      // for return to caller
+   struct jpeg_error_mgr pub; // "public" fields
+   std::jmp_buf setjmp_buffer;     // for return to caller
 };
 using my_error_ptr = struct my_error_mgr*;
 
-void my_error_exit( j_common_ptr cinfo ) {
+static void my_error_exit( j_common_ptr cinfo ) {
    // cinfo->err really points to a my_error_mgr struct, so coerce pointer
-   my_error_ptr myerr = reinterpret_cast<my_error_ptr>(cinfo->err);
+   my_error_ptr myerr = reinterpret_cast<my_error_ptr>( cinfo->err );
    // Return control to the setjmp point
    longjmp( myerr->setjmp_buffer, 1 );
 }
 
-void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
+static void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
+
+#define DIP__DECLARE_JPEG_EXIT( message ) \
+std::jmp_buf setjmp_buffer; if( setjmp( setjmp_buffer )) { DIP_THROW_RUNTIME( message ); }
+
+
+namespace dip {
+
+namespace {
 
 class JpegInput {
    public:
-      JpegInput( String filename ) : filename_( std::move( filename )) {
+      JpegInput( String filename, std::jmp_buf const& setjmp_buffer ) : filename_( std::move( filename )) {
          infile_ = std::fopen( filename_.c_str(), "rb" );
          if( infile_ == nullptr ) {
             if( !FileHasExtension( filename_ )) {
                filename_ = FileAddExtension( filename_, "jpg" ); // Try with "jpg" extension
                infile_ = std::fopen( filename_.c_str(), "rb" );
@@ -64,14 +73,11 @@ class JpegInput {
             DIP_THROW_RUNTIME( "Could not open the specified JPEG file" );
          }
          cinfo_.err = jpeg_std_error( &jerr_.pub );
          jerr_.pub.error_exit = my_error_exit;
          jerr_.pub.output_message = my_output_message;
-         if( setjmp( jerr_.setjmp_buffer )) {
-            // If we get here, the JPEG code has signaled an error.
-            DIP_THROW_RUNTIME( "Error reading JPEG file." );
-         }
+         std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
          jpeg_create_decompress( &cinfo_ );
          initialized_ = true;
          jpeg_stdio_src( &cinfo_, infile_ );
          jpeg_read_header( &cinfo_, TRUE );
       }
@@ -100,11 +106,11 @@ class JpegInput {
       bool initialized_ = false;
 };
 
 class JpegOutput {
    public:
-      explicit JpegOutput( String const& filename ) {
+      explicit JpegOutput( String const& filename, std::jmp_buf const& setjmp_buffer ) {
          // Open the file for writing
          if( FileHasExtension( filename )) {
             outfile_ = std::fopen(filename.c_str(), "wb");
          } else {
             outfile_ = std::fopen( FileAddExtension( filename, "jpg" ).c_str(), "wb" );
@@ -113,14 +119,11 @@ class JpegOutput {
             DIP_THROW_RUNTIME( "Could not open file for writing" );
          }
          cinfo_.err = jpeg_std_error( &jerr_.pub );
          jerr_.pub.error_exit = my_error_exit;
          jerr_.pub.output_message = my_output_message;
-         if( setjmp( jerr_.setjmp_buffer )) {
-            // If we get here, the JPEG code has signaled an error.
-            DIP_THROW_RUNTIME( "Error writing JPEG file." );
-         }
+         std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
          jpeg_create_compress( &cinfo_ );
          initialized_ = true;
          jpeg_stdio_dest( &cinfo_, outfile_ );
       }
       JpegOutput( JpegOutput const& ) = delete;
@@ -176,11 +179,12 @@ FileInformation GetJPEGInfo( JpegInput& jpeg ) {
 FileInformation ImageReadJPEG(
       Image& out,
       String const& filename
 ) {
    // Open the file
-   JpegInput jpeg( filename );
+   DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
+   JpegInput jpeg( filename, setjmp_buffer );
 
    // Get info
    FileInformation info = GetJPEGInfo( jpeg );
 
    // Allocate image
@@ -221,18 +225,20 @@ FileInformation ImageReadJPEG(
 
    return info;
 }
 
 FileInformation ImageReadJPEGInfo( String const& filename ) {
-   JpegInput jpeg( filename );
+   DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
+   JpegInput jpeg( filename, setjmp_buffer );
    FileInformation info = GetJPEGInfo( jpeg );
    return info;
 }
 
 bool ImageIsJPEG( String const& filename ) {
    try {
-      JpegInput jpeg( filename );
+      DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
+      JpegInput jpeg( filename, setjmp_buffer );
    } catch( ... ) {
       return false;
    }
    return true;
 }
@@ -242,23 +248,23 @@ void ImageWriteJPEG(
       String const& filename,
       dip::uint jpegLevel
 ) {
    DIP_THROW_IF( !image.IsForged(), E::IMAGE_NOT_FORGED );
    DIP_THROW_IF( image.Dimensionality() != 2, E::DIMENSIONALITY_NOT_SUPPORTED );
-   jpegLevel = clamp< dip::uint >( jpegLevel, 1, 100 );
 
    // Open the file
-   JpegOutput jpeg( filename );
+   DIP__DECLARE_JPEG_EXIT( "Error writing JPEG file" );
+   JpegOutput jpeg( filename, setjmp_buffer );
 
    // Set image properties
    int nchan = static_cast< int >( image.TensorElements() );
    jpeg.cinfo().image_width = static_cast< JDIMENSION >( image.Size( 0 ));
    jpeg.cinfo().image_height = static_cast< JDIMENSION >( image.Size( 1 ));
    jpeg.cinfo().input_components = nchan;
    jpeg.cinfo().in_color_space = nchan > 1 ? JCS_RGB : JCS_GRAYSCALE;
    jpeg_set_defaults( jpeg.cinfoptr() );
-   jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( jpegLevel ), FALSE );
+   jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( clamp< dip::uint >( jpegLevel, 1, 100 )), FALSE );
    jpeg.cinfo().density_unit = 2; // dots per cm
    jpeg.cinfo().X_density = static_cast< UINT16 >( 0.01 / image.PixelSize( 0 ).RemovePrefix().magnitude ); // let's assume it's meter
    jpeg.cinfo().Y_density = static_cast< UINT16 >( 0.01 / image.PixelSize( 1 ).RemovePrefix().magnitude );
 
    // Convert the image to uint8 if necessary

Vulnerable code: #include <setjmp.h>

namespace dip {

namespace {
   struct jpeg_error_mgr pub;   // "public" fields
   jmp_buf setjmp_buffer;      // for return to caller
void my_error_exit( j_common_ptr cinfo ) {
   my_error_ptr myerr = reinterpret_cast<my_error_ptr>(cinfo->err);
void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
      JpegInput( String filename ) : filename_( std::move( filename )) {
         if( setjmp( jerr_.setjmp_buffer )) {
            // If we get here, the JPEG code has signaled an error.
            DIP_THROW_RUNTIME( "Error reading JPEG file." );
         }
      explicit JpegOutput( String const& filename ) {
         if( setjmp( jerr_.setjmp_buffer )) {
            // If we get here, the JPEG code has signaled an error.
            DIP_THROW_RUNTIME( "Error writing JPEG file." );
         }
   JpegInput jpeg( filename );
   JpegInput jpeg( filename );
      JpegInput jpeg( filename );
   jpegLevel = clamp< dip::uint >( jpegLevel, 1, 100 );
   JpegOutput jpeg( filename );
   jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( jpegLevel ), FALSE );
Fixed code:    set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
   set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
   set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
- Fixed error handling in `dip::ImageReadJPEG()` and `dip::ImageWriteJPEG()`, which previously would crash when libjpeg produced an error (#80).

#include <csetjmp>
extern "C" {
   static void my_error_exit( j_common_ptr cinfo );
   static void my_output_message( j_common_ptr );
}

   struct jpeg_error_mgr pub; // "public" fields
   std::jmp_buf setjmp_buffer;     // for return to caller
static void my_error_exit( j_common_ptr cinfo ) {
   my_error_ptr myerr = reinterpret_cast<my_error_ptr>( cinfo->err );
static void my_output_message( j_common_ptr ) {} // Don't do anything with messages!

#define DIP__DECLARE_JPEG_EXIT( message ) \
std::jmp_buf setjmp_buffer; if( setjmp( setjmp_buffer )) { DIP_THROW_RUNTIME( message ); }


namespace dip {

namespace {
      JpegInput( String filename, std::jmp_buf const& setjmp_buffer ) : filename_( std::move( filename )) {
         std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
      explicit JpegOutput( String const& filename, std::jmp_buf const& setjmp_buffer ) {
         std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
   DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
   JpegInput jpeg( filename, setjmp_buffer );
   DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
   JpegInput jpeg( filename, setjmp_buffer );
      DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
      JpegInput jpeg( filename, setjmp_buffer );
   DIP__DECLARE_JPEG_EXIT( "Error writing JPEG file" );
   JpegOutput jpeg( filename, setjmp_buffer );
   jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( clamp< dip::uint >( jpegLevel, 1, 100 )), FALSE );