Skip to content

Fix EXIF bugs where corrupted exif blocks could overrun memory#3627

Merged
lgritz merged 1 commit intoAcademySoftwareFoundation:masterfrom
lgritz:lg-exifbug
Oct 22, 2022
Merged

Fix EXIF bugs where corrupted exif blocks could overrun memory#3627
lgritz merged 1 commit intoAcademySoftwareFoundation:masterfrom
lgritz:lg-exifbug

Conversation

@lgritz
Copy link
Copy Markdown
Collaborator

@lgritz lgritz commented Oct 22, 2022

In one case, we actually had a check for this, but an assignment to an int made the nonsensical offset appear negative, and we only tested whether the necessary offset it was bigger than the buffer size. Keeping it as (unsigned) size_t makes the test work as intended.

In another case, there were several places where we never checked that we were staying within the exif block, and here we address this by changing the utility decode_ifd so instead of passing it a pointer to the ifd, it passes the offset (the pointer turned out to always be inside the buffer) so it can check the extent for subsequent accesses.

Also some fixes related to squashing undefined behavior sanitizer cases.

In one case, we actually had a check for this, but an assignment to an
int made the nonsensical offset appear negative, and we only tested
whether the necessary offset it was bigger than the buffer size.
Keeping it as (unsigned) size_t makes the test work as intended.

In another case, there were several places where we never checked that
we were staying within the exif block, and here we address this by
changing the utility decode_ifd so instead of passing it a pointer to
the ifd, it passes the offset (the pointer turned out to always be
inside the buffer) so it can check the extent for subsequent accesses.

Also some fixes related to squashing undefined behavior sanitizer
cases.
@lgritz lgritz merged commit 884dfd6 into AcademySoftwareFoundation:master Oct 22, 2022
@lgritz lgritz deleted the lg-exifbug branch October 22, 2022 19:47
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Oct 22, 2022
…mySoftwareFoundation#3627)

In one case, we actually had a check for this, but an assignment to an
int made the nonsensical offset appear negative, and we only tested
whether the necessary offset it was bigger than the buffer size.
Keeping it as (unsigned) size_t makes the test work as intended.

In another case, there were several places where we never checked that
we were staying within the exif block, and here we address this by
changing the utility decode_ifd so instead of passing it a pointer to
the ifd, it passes the offset (the pointer turned out to always be
inside the buffer) so it can check the extent for subsequent accesses.

Also some fixes related to squashing undefined behavior sanitizer
cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant