Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix bug #65873 - Integer overflow in exif_read_data() #539

Closed
wants to merge 1 commit into from

3 participants

@smalyshev
Owner

No description provided.

@tstarling

I can confirm that it fixes our test case. But dir_entry-offset_base will be a ptrdiff_t, i.e. signed long, whereas dir_entry is unsigned. A TIFF file can be up to 4GB in length. So on a 32-bit system, you could imagine a case where dir_entry is near the end of a 4GB file, say 0xffff1000, and offset_base is near the start of addressable memory, say 0x1000. Then the subtraction will be negative and the condition will be false.

I think if you cast to unsigned, you would recover the actual file offset as intended, i.e.

            if (byte_count > IFDlength || offset_val > IFDlength-byte_count || value_ptr < dir_entry || offset_val < (size_t)(dir_entry-offset_base)) {

I've confirmed that that fixes our test case also.

@dsp
Owner

can this be closed?

@smalyshev
Owner

this one already merged

@smalyshev smalyshev closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 1 deletion.
  1. +3 −0  NEWS
  2. +6 −1 ext/exif/exif.c
View
3  NEWS
@@ -16,6 +16,9 @@ PHP NEWS
. Fixed bug #65196 (Passing DOMDocumentFragment to DOMDocument::saveHTML()
Produces invalid Markup). (Mike)
+- Exif:
+ . Fixed bug #65873 (Integer overflow in exif_read_data()). (Stas)
+
- Filter:
. Fixed bug #66229 (128.0.0.0/16 isn't reserved any longer). (Adam)
View
7 ext/exif/exif.c
@@ -2852,7 +2852,12 @@ static int exif_process_IFD_TAG(image_info_type *ImageInfo, char *dir_entry, cha
offset_val = php_ifd_get32u(dir_entry+8, ImageInfo->motorola_intel);
/* If its bigger than 4 bytes, the dir entry contains an offset. */
value_ptr = offset_base+offset_val;
- if (byte_count > IFDlength || offset_val > IFDlength-byte_count || value_ptr < dir_entry) {
+ /*
+ dir_entry is ImageInfo->file.list[sn].data+2+i*12
+ offset_base is ImageInfo->file.list[sn].data-dir_offset
+ dir_entry - offset_base is dir_offset+2+i*12
+ */
+ if (byte_count > IFDlength || offset_val > IFDlength-byte_count || value_ptr < dir_entry || offset_val < dir_entry-offset_base) {
/* It is important to check for IMAGE_FILETYPE_TIFF
* JPEG does not use absolute pointers instead its pointers are
* relative to the start of the TIFF header in APP1 section. */
Something went wrong with that request. Please try again.