gd-2.2.5-potential-double-free.patch

   1 From 4d9d8368d08c3a2be3ea4193b9314fffeddace52 Mon Sep 17 00:00:00 2001
   2 From: Ondrej Dubaj <odubaj@redhat.com>
   3 Date: Tue, 4 Jun 2019 13:38:41 +0200
   4 Subject: [PATCH] Potential double-free in gdImage*Ptr()
   5
   6 Whenever `gdImage*Ptr()` calls `gdImage*Ctx()` and the latter fails, we
   7 must not call `gdDPExtractData()`; otherwise a double-free would
   8 happen.  Since `gdImage*Ctx()` are void functions, and we can't change
   9 that for BC reasons, we're introducing static helpers which are used
  10 internally.
  11
  12 We're adding a regression test for `gdImageJpegPtr()`, but not for
  13 `gdImageGifPtr()` and `gdImageWbmpPtr()` since we don't know how to
  14 trigger failure of the respective `gdImage*Ctx()` calls.
  15
  16 This potential security issue has been reported by Solmaz Salimi (aka.
  17 Rooney).
  18 ---
  19  src/gd_gif_out.c                  | 19 +++++++++++++++----
  20  src/gd_jpeg.c                     | 20 ++++++++++++++++----
  21  src/gd_wbmp.c                     | 21 ++++++++++++++++++---
  22  tests/jpeg/CMakeLists.txt         |  1 +
  23  tests/jpeg/Makemodule.am          |  3 ++-
  24  tests/jpeg/jpeg_ptr_double_free.c | 31 +++++++++++++++++++++++++++++++
  25  6 files changed, 83 insertions(+), 12 deletions(-)
  26  create mode 100644 tests/jpeg/jpeg_ptr_double_free.c
  27
  28 diff --git a/src/gd_gif_out.c b/src/gd_gif_out.c
  29 index 6fe707d..4a05c09 100755
  30 --- a/src/gd_gif_out.c
  31 +++ b/src/gd_gif_out.c
  32 @@ -99,7 +99,7 @@ static void char_init(GifCtx *ctx);
  33  static void char_out(int c, GifCtx *ctx);
  34  static void flush_char(GifCtx *ctx);
  35
  36 -
  37 +static int _gdImageGifCtx(gdImagePtr im, gdIOCtxPtr out);
  38
  39
  40  /*
  41 @@ -131,8 +131,11 @@ BGD_DECLARE(void *) gdImageGifPtr(gdImagePtr im, int *size)
  42         void *rv;
  43         gdIOCtx *out = gdNewDynamicCtx(2048, NULL);
  44         if (out == NULL) return NULL;
  45 -       gdImageGifCtx(im, out);
  46 -       rv = gdDPExtractData(out, size);
  47 +       if (!_gdImageGifCtx(im, out)) {
  48 +        rv = gdDPExtractData(out, size);
  49 +    } else {
  50 +        rv = NULL;
  51 +    }
  52         out->gd_free(out);
  53         return rv;
  54  }
  55 @@ -220,6 +223,12 @@ BGD_DECLARE(void) gdImageGif(gdImagePtr im, FILE *outFile)
  56
  57  */
  58  BGD_DECLARE(void) gdImageGifCtx(gdImagePtr im, gdIOCtxPtr out)
  59 +{
  60 +    _gdImageGifCtx(im, out);
  61 +}
  62 +
  63 +/* returns 0 on success, 1 on failure */
  64 +static int _gdImageGifCtx(gdImagePtr im, gdIOCtxPtr out)
  65  {
  66         gdImagePtr pim = 0, tim = im;
  67         int interlace, BitsPerPixel;
  68 @@ -231,7 +240,7 @@ BGD_DECLARE(void) gdImageGifCtx(gdImagePtr im, gdIOCtxPtr out)
  69                 based temporary image. */
  70                 pim = gdImageCreatePaletteFromTrueColor(im, 1, 256);
  71                 if(!pim) {
  72 -                       return;
  73 +                       return 1;
  74                 }
  75                 tim = pim;
  76         }
  77 @@ -247,6 +256,8 @@ BGD_DECLARE(void) gdImageGifCtx(gdImagePtr im, gdIOCtxPtr out)
  78                 /* Destroy palette based temporary image. */
  79                 gdImageDestroy( pim);
  80         }
  81 +
  82 +    return 0;
  83  }
  84
  85
  86 diff --git a/src/gd_jpeg.c b/src/gd_jpeg.c
  87 index 271ef46..bd8fc27 100755
  88 --- a/src/gd_jpeg.c
  89 +++ b/src/gd_jpeg.c
  90 @@ -123,6 +123,8 @@ static void fatal_jpeg_error(j_common_ptr cinfo)
  91         exit(99);
  92  }
  93
  94 +static int _gdImageJpegCtx(gdImagePtr im, gdIOCtx *outfile, int quality);
  95 +
  96  /*
  97   * Write IM to OUTFILE as a JFIF-formatted JPEG image, using quality
  98   * QUALITY.  If QUALITY is in the range 0-100, increasing values
  99 @@ -237,8 +239,11 @@ BGD_DECLARE(void *) gdImageJpegPtr(gdImagePtr im, int *size, int quality)
 100         void *rv;
 101         gdIOCtx *out = gdNewDynamicCtx(2048, NULL);
 102         if (out == NULL) return NULL;
 103 -       gdImageJpegCtx(im, out, quality);
 104 -       rv = gdDPExtractData(out, size);
 105 +       if (!_gdImageJpegCtx(im, out, quality)) {
 106 +               rv = gdDPExtractData(out, size);
 107 +       } else {
 108 +               rv = NULL;
 109 +       }
 110         out->gd_free(out);
 111         return rv;
 112  }
 113 @@ -259,6 +264,12 @@ void jpeg_gdIOCtx_dest(j_compress_ptr cinfo, gdIOCtx *outfile);
 114
 115  */
 116  BGD_DECLARE(void) gdImageJpegCtx(gdImagePtr im, gdIOCtx *outfile, int quality)
 117 +{
 118 +       _gdImageJpegCtx(im, outfile, quality);
 119 +}
 120 +
 121 +/* returns 0 on success, 1 on failure */
 122 +static int _gdImageJpegCtx(gdImagePtr im, gdIOCtx *outfile, int quality)
 123  {
 124         struct jpeg_compress_struct cinfo;
 125         struct jpeg_error_mgr jerr;
 126 @@ -293,7 +304,7 @@ BGD_DECLARE(void) gdImageJpegCtx(gdImagePtr im, gdIOCtx *outfile, int quality)
 127                 if(row) {
 128                         gdFree(row);
 129                 }
 130 -               return;
 131 +               return 1;
 132         }
 133
 134         cinfo.err->emit_message = jpeg_emit_message;
 135 @@ -334,7 +345,7 @@ BGD_DECLARE(void) gdImageJpegCtx(gdImagePtr im, gdIOCtx *outfile, int quality)
 136         if(row == 0) {
 137                 gd_error("gd-jpeg: error: unable to allocate JPEG row structure: gdCalloc returns NULL\n");
 138                 jpeg_destroy_compress(&cinfo);
 139 -               return;
 140 +               return 1;
 141         }
 142
 143         rowptr[0] = row;
 144 @@ -411,6 +422,7 @@ BGD_DECLARE(void) gdImageJpegCtx(gdImagePtr im, gdIOCtx *outfile, int quality)
 145         jpeg_finish_compress(&cinfo);
 146         jpeg_destroy_compress(&cinfo);
 147         gdFree(row);
 148 +       return 0;
 149  }
 150
 151
 152 diff --git a/src/gd_wbmp.c b/src/gd_wbmp.c
 153 index 0028273..341ff6e 100755
 154 --- a/src/gd_wbmp.c
 155 +++ b/src/gd_wbmp.c
 156 @@ -88,6 +88,8 @@ int gd_getin(void *in)
 157         return (gdGetC((gdIOCtx *)in));
 158  }
 159
 160 +static int _gdImageWBMPCtx(gdImagePtr image, int fg, gdIOCtx *out);
 161 +
 162  /*
 163         Function: gdImageWBMPCtx
 164
 165 @@ -100,6 +102,12 @@ int gd_getin(void *in)
 166                 out   - the stream where to write
 167  */
 168  BGD_DECLARE(void) gdImageWBMPCtx(gdImagePtr image, int fg, gdIOCtx *out)
 169 +{
 170 +       _gdImageWBMPCtx(image, fg, out);
 171 +}
 172 +
 173 +/* returns 0 on success, 1 on failure */
 174 +static int _gdImageWBMPCtx(gdImagePtr image, int fg, gdIOCtx *out)
 175  {
 176         int x, y, pos;
 177         Wbmp *wbmp;
 178 @@ -107,7 +115,7 @@ BGD_DECLARE(void) gdImageWBMPCtx(gdImagePtr image, int fg, gdIOCtx *out)
 179         /* create the WBMP */
 180         if((wbmp = createwbmp(gdImageSX(image), gdImageSY(image), WBMP_WHITE)) == NULL) {
 181                 gd_error("Could not create WBMP\n");
 182 -               return;
 183 +               return 1;
 184         }
 185
 186         /* fill up the WBMP structure */
 187 @@ -123,11 +131,15 @@ BGD_DECLARE(void) gdImageWBMPCtx(gdImagePtr image, int fg, gdIOCtx *out)
 188
 189         /* write the WBMP to a gd file descriptor */
 190         if(writewbmp(wbmp, &gd_putout, out)) {
 191 +               freewbmp(wbmp);
 192                 gd_error("Could not save WBMP\n");
 193 +               return 1;
 194         }
 195
 196         /* des submitted this bugfix: gdFree the memory. */
 197         freewbmp(wbmp);
 198 +
 199 +       return 0;
 200  }
 201
 202  /*
 203 @@ -271,8 +283,11 @@ BGD_DECLARE(void *) gdImageWBMPPtr(gdImagePtr im, int *size, int fg)
 204         void *rv;
 205         gdIOCtx *out = gdNewDynamicCtx(2048, NULL);
 206         if (out == NULL) return NULL;
 207 -       gdImageWBMPCtx(im, fg, out);
 208 -       rv = gdDPExtractData(out, size);
 209 +       if (!_gdImageWBMPCtx(im, fg, out)) {
 210 +               rv = gdDPExtractData(out, size);
 211 +       } else {
 212 +               rv = NULL;
 213 +       }
 214         out->gd_free(out);
 215         return rv;
 216  }
 217 diff --git a/tests/jpeg/CMakeLists.txt b/tests/jpeg/CMakeLists.txt
 218 index 19964b0..a8d8162 100755
 219 --- a/tests/jpeg/CMakeLists.txt
 220 +++ b/tests/jpeg/CMakeLists.txt
 221 @@ -2,6 +2,7 @@ IF(JPEG_FOUND)
 222  LIST(APPEND TESTS_FILES
 223         jpeg_empty_file
 224         jpeg_im2im
 225 +       jpeg_ptr_double_free
 226         jpeg_null
 227  )
 228
 229 diff --git a/tests/jpeg/Makemodule.am b/tests/jpeg/Makemodule.am
 230 index 7e5d317..b89e169 100755
 231 --- a/tests/jpeg/Makemodule.am
 232 +++ b/tests/jpeg/Makemodule.am
 233 @@ -2,7 +2,8 @@ if HAVE_LIBJPEG
 234  libgd_test_programs += \
 235         jpeg/jpeg_empty_file \
 236         jpeg/jpeg_im2im \
 237 -       jpeg/jpeg_null
 238 +       jpeg/jpeg_null \
 239 +       jpeg/jpeg_ptr_double_free
 240
 241  if HAVE_LIBPNG
 242  libgd_test_programs += \
 243 diff --git a/tests/jpeg/jpeg_ptr_double_free.c b/tests/jpeg/jpeg_ptr_double_free.c
 244 new file mode 100644
 245 index 0000000..c80aeb6
 246 --- /dev/null
 247 +++ b/tests/jpeg/jpeg_ptr_double_free.c
 248 @@ -0,0 +1,31 @@
 249 +/**
 250 + * Test that failure to convert to JPEG returns NULL
 251 + *
 252 + * We are creating an image, set its width to zero, and pass this image to
 253 + * `gdImageJpegPtr()` which is supposed to fail, and as such should return NULL.
 254 + *
 255 + * See also <https://github.com/libgd/libgd/issues/381>
 256 + */
 257 +
 258 +
 259 +#include "gd.h"
 260 +#include "gdtest.h"
 261 +
 262 +
 263 +int main()
 264 +{
 265 +    gdImagePtr src, dst;
 266 +    int size;
 267 +
 268 +    src = gdImageCreateTrueColor(1, 10);
 269 +    gdTestAssert(src != NULL);
 270 +
 271 +    src->sx = 0; /* this hack forces gdImageJpegPtr() to fail */
 272 +
 273 +    dst = gdImageJpegPtr(src, &size, 0);
 274 +    gdTestAssert(dst == NULL);
 275 +
 276 +    gdImageDestroy(src);
 277 +
 278 +    return gdNumFailures();
 279 +}
 280 \ No newline at end of file
 281 --
 282 2.17.1
 283