Make MAX_IFD_NESTING_LEVEL an actual nesting level

Currently we only ever increment ifd_nesting_level, so this ends up
being a limit on the total number of IFD tags and we regularly get
bug reports of it being exceeded. I think the intention behind this
limit was to prevent recursion stack overflow, and for that we only
need to check actual recursive usage. I've implemented that here,
and dropped the nesting limit down to a smaller value
(which still passes our tests).

However, it seems that we do also need to have a total limit on
the number of tags, as we don't catch some instances of infinite
looping otherwise. Add this as a separate limit with a higher
value, that should hopefully be sufficient.

This is expected to fix a number of bugs:

https://bugs.php.net/bug.php?id=78083
https://bugs.php.net/bug.php?id=78701
https://bugs.php.net/bug.php?id=79907
https://bugs.php.net/bug.php?id=80016

diff --git a/ext/exif/exif.c b/ext/exif/exif.c
index 5e4c23f4e623bb741ad630e3dc2bd7c09af6305d..1ebc972d8d44c38e3764fd2fcbb7aa3ca4a9e934 100644 (file)
--- a/ext/exif/exif.c
+++ b/ext/exif/exif.c
@@ -66,7 +66,8 @@ typedef unsigned char uchar;
 
 #define EFREE_IF(ptr)  if (ptr) efree(ptr)
 
-#define MAX_IFD_NESTING_LEVEL 200
+#define MAX_IFD_NESTING_LEVEL 10
+#define MAX_IFD_TAGS 1000
 
 /* {{{ arginfo */
 ZEND_BEGIN_ARG_INFO(arginfo_exif_tagname, 0)
@@ -1940,6 +1941,7 @@ typedef struct {
        int             read_thumbnail;
        int             read_all;
        int             ifd_nesting_level;
+       int             ifd_count;
        /* internal */
        file_section_list       file;
 } image_info_type;
@@ -2674,6 +2676,7 @@ static void exif_process_SOFn (uchar *Data, int marker, jpeg_sof_info *result)
 /* forward declarations */
 static int exif_process_IFD_in_JPEG(image_info_type *ImageInfo, char *dir_start, char *offset_base, size_t IFDlength, size_t displacement, int section_index, int tag);
 static int exif_process_IFD_TAG(    image_info_type *ImageInfo, char *dir_entry, char *offset_base, size_t IFDlength, size_t displacement, int section_index, int ReadNextIFD, tag_table_type tag_table);
+static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offset, int section_index);
 
 /* {{{ exif_get_markername
        Get name of marker */
@@ -3246,7 +3249,7 @@ static int exif_process_IFD_in_MAKERNOTE(image_info_type *ImageInfo, char * valu
 
 /* {{{ exif_process_IFD_TAG
  * Process one of the nested IFDs directories. */
-static int exif_process_IFD_TAG(image_info_type *ImageInfo, char *dir_entry, char *offset_base, size_t IFDlength, size_t displacement, int section_index, int ReadNextIFD, tag_table_type tag_table)
+static int exif_process_IFD_TAG_impl(image_info_type *ImageInfo, char *dir_entry, char *offset_base, size_t IFDlength, size_t displacement, int section_index, int ReadNextIFD, tag_table_type tag_table)
 {
        size_t length;
        unsigned int tag, format, components;
@@ -3259,13 +3262,6 @@ static int exif_process_IFD_TAG(image_info_type *ImageInfo, char *dir_entry, cha
        int dump_free;
 #endif /* EXIF_DEBUG */
 
-       /* Protect against corrupt headers */
-       if (ImageInfo->ifd_nesting_level > MAX_IFD_NESTING_LEVEL) {
-               exif_error_docref("exif_read_data#error_ifd" EXIFERR_CC, ImageInfo, E_WARNING, "corrupt EXIF header: maximum directory nesting level reached");
-               return FALSE;
-       }
-       ImageInfo->ifd_nesting_level++;
-
        tag = php_ifd_get16u(dir_entry, ImageInfo->motorola_intel);
        format = php_ifd_get16u(dir_entry+2, ImageInfo->motorola_intel);
        components = php_ifd_get32u(dir_entry+4, ImageInfo->motorola_intel);
@@ -3585,6 +3581,24 @@ static int exif_process_IFD_TAG(image_info_type *ImageInfo, char *dir_entry, cha
 }
 /* }}} */
 
+static int exif_process_IFD_TAG(image_info_type *ImageInfo, char *dir_entry, char *offset_base, size_t IFDlength, size_t displacement, int section_index, int ReadNextIFD, tag_table_type tag_table)
+{
+       int result;
+       /* Protect against corrupt headers */
+       if (ImageInfo->ifd_count++ > MAX_IFD_TAGS) {
+               exif_error_docref("exif_read_data#error_ifd" EXIFERR_CC, ImageInfo, E_WARNING, "corrupt EXIF header: maximum IFD tag count reached");
+               return FALSE;
+       }
+       if (ImageInfo->ifd_nesting_level > MAX_IFD_NESTING_LEVEL) {
+               exif_error_docref("exif_read_data#error_ifd" EXIFERR_CC, ImageInfo, E_WARNING, "corrupt EXIF header: maximum directory nesting level reached");
+               return FALSE;
+       }
+       ImageInfo->ifd_nesting_level++;
+       result = exif_process_IFD_TAG_impl(ImageInfo, dir_entry, offset_base, IFDlength, displacement, section_index, ReadNextIFD, tag_table);
+       ImageInfo->ifd_nesting_level--;
+       return result;
+}
+
 /* {{{ exif_process_IFD_in_JPEG
  * Process one of the nested IFDs directories. */
 static int exif_process_IFD_in_JPEG(image_info_type *ImageInfo, char *dir_start, char *offset_base, size_t IFDlength, size_t displacement, int section_index, int tag)
@@ -4018,7 +4032,7 @@ static int exif_scan_thumbnail(image_info_type *ImageInfo)
 
 /* {{{ exif_process_IFD_in_TIFF
  * Parse the TIFF header; */
-static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offset, int section_index)
+static int exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir_offset, int section_index)
 {
        int i, sn, num_entries, sub_section_index = 0;
        unsigned char *dir_entry;
@@ -4027,10 +4041,6 @@ static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offse
        int entry_tag , entry_type;
        tag_table_type tag_table = exif_get_tag_table(section_index);
 
-       if (ImageInfo->ifd_nesting_level > MAX_IFD_NESTING_LEVEL) {
-               return FALSE;
-       }
-
        if (ImageInfo->FileSize >= 2 && ImageInfo->FileSize - 2 >= dir_offset) {
                sn = exif_file_sections_add(ImageInfo, M_PSEUDO, 2, NULL);
 #ifdef EXIF_DEBUG
@@ -4174,7 +4184,6 @@ static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offse
 #ifdef EXIF_DEBUG
                                                exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Next IFD: %s @x%04X", exif_get_sectionname(sub_section_index), entry_offset);
 #endif
-                                               ImageInfo->ifd_nesting_level++;
                                                exif_process_IFD_in_TIFF(ImageInfo, entry_offset, sub_section_index);
                                                if (section_index!=SECTION_THUMBNAIL && entry_tag==TAG_SUB_IFD) {
                                                        if (ImageInfo->Thumbnail.filetype != IMAGE_FILETYPE_UNKNOWN
@@ -4218,7 +4227,6 @@ static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offse
 #ifdef EXIF_DEBUG
                                        exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Read next IFD (THUMBNAIL) at x%04X", next_offset);
 #endif
-                                       ImageInfo->ifd_nesting_level++;
                                        exif_process_IFD_in_TIFF(ImageInfo, next_offset, SECTION_THUMBNAIL);
 #ifdef EXIF_DEBUG
                                        exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "%s THUMBNAIL @0x%04X + 0x%04X", ImageInfo->Thumbnail.data ? "Ignore" : "Read", ImageInfo->Thumbnail.offset, ImageInfo->Thumbnail.size);
@@ -4255,6 +4263,21 @@ static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offse
 }
 /* }}} */
 
+static int exif_process_IFD_in_TIFF(image_info_type *ImageInfo, size_t dir_offset, int section_index)
+{
+       int result;
+       if (ImageInfo->ifd_count++ > MAX_IFD_TAGS) {
+               return FALSE;
+       }
+       if (ImageInfo->ifd_nesting_level > MAX_IFD_NESTING_LEVEL) {
+               return FALSE;
+       }
+       ImageInfo->ifd_nesting_level++;
+       result = exif_process_IFD_in_TIFF_impl(ImageInfo, dir_offset, section_index);
+       ImageInfo->ifd_nesting_level--;
+       return result;
+}
+
 /* {{{ exif_scan_FILE_header
  * Parse the marker stream until SOS or EOI is seen; */
 static int exif_scan_FILE_header(image_info_type *ImageInfo)
@@ -4410,6 +4433,7 @@ static int exif_read_from_impl(image_info_type *ImageInfo, php_stream *stream, i
 
 
        ImageInfo->ifd_nesting_level = 0;
+       ImageInfo->ifd_count = 0;
 
        /* Scan the headers */
        ret = exif_scan_FILE_header(ImageInfo);

diff --git a/ext/exif/tests/nesting_level_oom.phpt b/ext/exif/tests/nesting_level_oom.phpt
new file mode 100644 (file)
index 0000000..bde1f8f
--- /dev/null
+++ b/ext/exif/tests/nesting_level_oom.phpt
@@ -0,0 +1,10 @@
+--TEST--
+Should not cause OOM
+--FILE--
+<?php
+
+var_dump(@exif_read_data(__DIR__ . '/nesting_level_oom.tiff'));
+
+?>
+--EXPECT--
+bool(false)

diff --git a/ext/exif/tests/nesting_level_oom.tiff b/ext/exif/tests/nesting_level_oom.tiff
new file mode 100644 (file)
index 0000000..9bdcdc0
Binary files /dev/null and b/ext/exif/tests/nesting_level_oom.tiff differ