[Bf-blender-cvs] [15ffda3bcd6] master: Fix T82602: checking image header reads past buffer bounds

Campbell Barton noreply at git.blender.org
Wed Nov 11 06:15:00 CET 2020


Commit: 15ffda3bcd697e6f3a0cc13e141da865f36f3b53
Author: Campbell Barton
Date:   Wed Nov 11 16:14:09 2020 +1100
Branches: master
https://developer.blender.org/rB15ffda3bcd697e6f3a0cc13e141da865f36f3b53

Fix T82602: checking image header reads past buffer bounds

Use the size argument to ensure checking the header doesn't read
past the buffer bounds when reading corrupt/truncated headers
from image files.

===================================================================

M	source/blender/imbuf/intern/bmp.c
M	source/blender/imbuf/intern/cineon/cineon_dpx.c
M	source/blender/imbuf/intern/cineon/logImageCore.c
M	source/blender/imbuf/intern/cineon/logImageCore.h
M	source/blender/imbuf/intern/dds/dds_api.cpp
M	source/blender/imbuf/intern/iris.c
M	source/blender/imbuf/intern/jp2.c
M	source/blender/imbuf/intern/jpeg.c
M	source/blender/imbuf/intern/oiio/openimageio_api.cpp
M	source/blender/imbuf/intern/openexr/openexr_api.cpp
M	source/blender/imbuf/intern/png.c
M	source/blender/imbuf/intern/radiance_hdr.c
M	source/blender/imbuf/intern/targa.c
M	source/blender/imbuf/intern/tiff.c

===================================================================

diff --git a/source/blender/imbuf/intern/bmp.c b/source/blender/imbuf/intern/bmp.c
index 9358b67b3ed..58ce02f28ae 100644
--- a/source/blender/imbuf/intern/bmp.c
+++ b/source/blender/imbuf/intern/bmp.c
@@ -72,10 +72,14 @@ typedef struct BMPHEADER {
    CHECK_HEADER_FIELD(_mem, "CI") || CHECK_HEADER_FIELD(_mem, "CP") || \
    CHECK_HEADER_FIELD(_mem, "IC") || CHECK_HEADER_FIELD(_mem, "PT"))
 
-static bool checkbmp(const uchar *mem)
+static bool checkbmp(const uchar *mem, const size_t size)
 {
+  if (size < BMP_FILEHEADER_SIZE) {
+    return false;
+  }
+
   if (!CHECK_HEADER_FIELD_BMP(mem)) {
-    return 0;
+    return false;
   }
 
   bool ok = false;
@@ -102,9 +106,9 @@ static bool checkbmp(const uchar *mem)
   return ok;
 }
 
-bool imb_is_a_bmp(const uchar *buf, size_t UNUSED(size))
+bool imb_is_a_bmp(const uchar *buf, size_t size)
 {
-  return checkbmp(buf);
+  return checkbmp(buf, size);
 }
 
 ImBuf *imb_bmp_decode(const uchar *mem, size_t size, int flags, char colorspace[IM_MAX_SPACE])
@@ -120,7 +124,7 @@ ImBuf *imb_bmp_decode(const uchar *mem, size_t size, int flags, char colorspace[
 
   (void)size; /* unused */
 
-  if (checkbmp(mem) == 0) {
+  if (checkbmp(mem, size) == 0) {
     return NULL;
   }
 
diff --git a/source/blender/imbuf/intern/cineon/cineon_dpx.c b/source/blender/imbuf/intern/cineon/cineon_dpx.c
index 8bbd9fbb285..de54e6dab9d 100644
--- a/source/blender/imbuf/intern/cineon/cineon_dpx.c
+++ b/source/blender/imbuf/intern/cineon/cineon_dpx.c
@@ -188,9 +188,9 @@ bool imb_save_cineon(struct ImBuf *buf, const char *filepath, int flags)
   return imb_save_dpx_cineon(buf, filepath, 1, flags);
 }
 
-bool imb_is_a_cineon(const unsigned char *buf, size_t UNUSED(size))
+bool imb_is_a_cineon(const unsigned char *buf, size_t size)
 {
-  return logImageIsCineon(buf);
+  return logImageIsCineon(buf, size);
 }
 
 ImBuf *imb_load_cineon(const unsigned char *mem,
@@ -209,9 +209,9 @@ bool imb_save_dpx(struct ImBuf *buf, const char *filepath, int flags)
   return imb_save_dpx_cineon(buf, filepath, 0, flags);
 }
 
-bool imb_is_a_dpx(const unsigned char *buf, size_t UNUSED(size))
+bool imb_is_a_dpx(const unsigned char *buf, size_t size)
 {
-  return logImageIsDpx(buf);
+  return logImageIsDpx(buf, size);
 }
 
 ImBuf *imb_load_dpx(const unsigned char *mem,
diff --git a/source/blender/imbuf/intern/cineon/logImageCore.c b/source/blender/imbuf/intern/cineon/logImageCore.c
index d5f5691c5f0..2d42609fdf5 100644
--- a/source/blender/imbuf/intern/cineon/logImageCore.c
+++ b/source/blender/imbuf/intern/cineon/logImageCore.c
@@ -96,15 +96,23 @@ void logImageSetVerbose(int verbosity)
  * IO stuff
  */
 
-int logImageIsDpx(const void *buffer)
+int logImageIsDpx(const void *buffer, const unsigned int size)
 {
-  unsigned int magicNum = *(unsigned int *)buffer;
+  unsigned int magicNum;
+  if (size < sizeof(magicNum)) {
+    return 0;
+  }
+  magicNum = *(unsigned int *)buffer;
   return (magicNum == DPX_FILE_MAGIC || magicNum == swap_uint(DPX_FILE_MAGIC, 1));
 }
 
-int logImageIsCineon(const void *buffer)
+int logImageIsCineon(const void *buffer, const unsigned int size)
 {
-  unsigned int magicNum = *(unsigned int *)buffer;
+  unsigned int magicNum;
+  if (size < sizeof(magicNum)) {
+    return 0;
+  }
+  magicNum = *(unsigned int *)buffer;
   return (magicNum == CINEON_FILE_MAGIC || magicNum == swap_uint(CINEON_FILE_MAGIC, 1));
 }
 
@@ -119,17 +127,17 @@ LogImageFile *logImageOpenFromFile(const char *filename, int cineon)
     return NULL;
   }
 
-  if (fread(&magicNum, sizeof(unsigned int), 1, f) != 1) {
+  if (fread(&magicNum, sizeof(magicNum), 1, f) != 1) {
     fclose(f);
     return NULL;
   }
 
   fclose(f);
 
-  if (logImageIsDpx(&magicNum)) {
+  if (logImageIsDpx(&magicNum, sizeof(magicNum))) {
     return dpxOpen((const unsigned char *)filename, 0, 0);
   }
-  if (logImageIsCineon(&magicNum)) {
+  if (logImageIsCineon(&magicNum, sizeof(magicNum))) {
     return cineonOpen((const unsigned char *)filename, 0, 0);
   }
 
@@ -138,10 +146,10 @@ LogImageFile *logImageOpenFromFile(const char *filename, int cineon)
 
 LogImageFile *logImageOpenFromMemory(const unsigned char *buffer, unsigned int size)
 {
-  if (logImageIsDpx(buffer)) {
+  if (logImageIsDpx(buffer, size)) {
     return dpxOpen(buffer, 1, size);
   }
-  if (logImageIsCineon(buffer)) {
+  if (logImageIsCineon(buffer, size)) {
     return cineonOpen(buffer, 1, size);
   }
 
diff --git a/source/blender/imbuf/intern/cineon/logImageCore.h b/source/blender/imbuf/intern/cineon/logImageCore.h
index a2d50f21a98..c2704a086b6 100644
--- a/source/blender/imbuf/intern/cineon/logImageCore.h
+++ b/source/blender/imbuf/intern/cineon/logImageCore.h
@@ -181,8 +181,8 @@ enum descriptor {
 /* int functions return 0 for OK */
 
 void logImageSetVerbose(int verbosity);
-int logImageIsDpx(const void *buffer);
-int logImageIsCineon(const void *buffer);
+int logImageIsDpx(const void *buffer, unsigned int size);
+int logImageIsCineon(const void *buffer, unsigned int size);
 LogImageFile *logImageOpenFromMemory(const unsigned char *buffer, unsigned int size);
 LogImageFile *logImageOpenFromFile(const char *filename, int cineon);
 void logImageGetSize(LogImageFile *logImage, int *width, int *height, int *depth);
diff --git a/source/blender/imbuf/intern/dds/dds_api.cpp b/source/blender/imbuf/intern/dds/dds_api.cpp
index 78ce8b5ee9b..5687824f9fd 100644
--- a/source/blender/imbuf/intern/dds/dds_api.cpp
+++ b/source/blender/imbuf/intern/dds/dds_api.cpp
@@ -73,8 +73,11 @@ bool imb_save_dds(struct ImBuf *ibuf, const char *name, int /*flags*/)
 }
 
 /* note: use at most first 32 bytes */
-bool imb_is_a_dds(const unsigned char *mem, size_t UNUSED(size))
+bool imb_is_a_dds(const unsigned char *mem, const size_t size)
 {
+  if (size < 8) {
+    return false;
+  }
   /* heuristic check to see if mem contains a DDS file */
   /* header.fourcc == FOURCC_DDS */
   if ((mem[0] != 'D') || (mem[1] != 'D') || (mem[2] != 'S') || (mem[3] != ' ')) {
diff --git a/source/blender/imbuf/intern/iris.c b/source/blender/imbuf/intern/iris.c
index c27ac5754c7..112b95bf1a1 100644
--- a/source/blender/imbuf/intern/iris.c
+++ b/source/blender/imbuf/intern/iris.c
@@ -243,8 +243,11 @@ static void test_endian_zbuf(struct ImBuf *ibuf)
 /* this one is only def-ed once, strangely... */
 #define GSS(x) (((uchar *)(x))[1] << 8 | ((uchar *)(x))[0])
 
-bool imb_is_a_iris(const uchar *mem, size_t UNUSED(size))
+bool imb_is_a_iris(const uchar *mem, size_t size)
 {
+  if (size < 2) {
+    return false;
+  }
   return ((GS(mem) == IMAGIC) || (GSS(mem) == IMAGIC));
 }
 
diff --git a/source/blender/imbuf/intern/jp2.c b/source/blender/imbuf/intern/jp2.c
index b964510b8db..e19589317d7 100644
--- a/source/blender/imbuf/intern/jp2.c
+++ b/source/blender/imbuf/intern/jp2.c
@@ -58,31 +58,38 @@ enum {
   DCP_CINEMA4K = 4,
 };
 
-static bool check_jp2(const unsigned char *mem) /* J2K_CFMT */
+static bool check_jp2(const unsigned char *mem, const size_t size) /* J2K_CFMT */
 {
+  if (size < sizeof(JP2_HEAD)) {
+    return false;
+  }
   return memcmp(JP2_HEAD, mem, sizeof(JP2_HEAD)) ? 0 : 1;
 }
 
-static bool check_j2k(const unsigned char *mem) /* J2K_CFMT */
+static bool check_j2k(const unsigned char *mem, const size_t size) /* J2K_CFMT */
 {
+  if (size < sizeof(J2K_HEAD)) {
+    return false;
+  }
   return memcmp(J2K_HEAD, mem, sizeof(J2K_HEAD)) ? 0 : 1;
 }
 
-static OPJ_CODEC_FORMAT format_from_header(const unsigned char mem[JP2_FILEHEADER_SIZE])
+static OPJ_CODEC_FORMAT format_from_header(const unsigned char mem[JP2_FILEHEADER_SIZE],
+                                           const size_t size)
 {
-  if (check_jp2(mem)) {
+  if (check_jp2(mem, size)) {
     return OPJ_CODEC_JP2;
   }
-  if (check_j2k(mem)) {
+  if (check_j2k(mem, size)) {
     return OPJ_CODEC_J2K;
   }
 
   return OPJ_CODEC_UNKNOWN;
 }
 
-bool imb_is_a_jp2(const unsigned char *buf, size_t UNUSED(size))
+bool imb_is_a_jp2(const unsigned char *buf, size_t size)
 {
-  return (check_jp2(buf) || check_j2k(buf));
+  return (check_jp2(buf, size) || check_j2k(buf, size));
 }
 
 /**
@@ -317,7 +324,7 @@ ImBuf *imb_load_jp2(const unsigned char *mem,
                     int flags,
                     char colorspace[IM_MAX_SPACE])
 {
-  const OPJ_CODEC_FORMAT format = (size > JP2_FILEHEADER_SIZE) ? format_from_header(mem) :
+  const OPJ_CODEC_FORMAT format = (size > JP2_FILEHEADER_SIZE) ? format_from_header(mem, size) :
                                                                  OPJ_CODEC_UNKNOWN;
   struct BufInfo buf_wrapper = {
       .buf = mem,
@@ -348,7 +355,7 @@ ImBuf *imb_load_jp2_filepath(const char *filepath, int flags, char colorspace[IM
 
   fseek(p_file, 0, SEEK_SET);
 
-  const OPJ_CODEC_FORMAT format = format_from_header(mem);
+  const OPJ_CODEC_FORMAT format = format_from_header(mem, sizeof(mem));
   ImBuf *ibuf = imb_load_jp2_stream(stream, format, flags, colorspace);
   opj_stream_destroy(stream);
   return ibuf;
diff --git a/source/blender/imbuf/intern/jpeg.c b/source/blender/imbuf/intern/jpeg.c
index 4a937738b52..93cdbbb1407 100644
--- a/source/blender/imbuf/intern/jpeg.c
+++ b/source/blender/imbuf/intern/jpeg.c
@@ -57,12 +57,13 @@ static ImBuf *ibJpegImageFromCinfo(struct jpeg_decompress_struct *cinfo, int fla
 static const uchar jpeg_default_quality = 75;
 static uchar ibuf_quality;
 
-bool imb_is_a_jpeg(const unsigned char *mem, const size_t UNUSED(size))
+bool imb_is_a_jpeg(const unsigned char *mem, const size_t size)
 {
-  if ((mem[0] == 0xFF) && (mem[1] == 0xD8)) {
-    return 1;
+  const char magic[2] = {0xFF, 0xD8};
+  if (size < sizeof(magic)) {
+    return false;
   }
-  return 0;
+  return memcmp(mem, magic, sizeof(magic)) == 0;
 }
 
 /*----------------------------------------------------------
diff --git a/source/blender/imbuf/intern/oiio/openimageio_api.cpp b/source/blender/imbuf/intern/oiio/openimageio_api.cpp
index 9f27ac96e6d..1e8c3c25778 100644
--- a/source/blender/imbuf/intern/oiio/openimageio_api.cpp
+++ b/source/blender/imbuf/intern/oiio/openimageio_api.cpp
@@ -163,9 +163,12 @@ static ImBuf *imb_oiio_load_image_float(
 
 extern "C" {
 
-bool imb_is_a_photoshop(const unsigned char *mem, size_t UNUSED(size))
+bool imb_is_a_photoshop(const unsigned char *mem, size_t si

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list