Skip to content

Harden gain-map pixel count arithmetic and dimension validation#3220

Closed
metsw24-max wants to merge 1 commit into
AOMediaCodec:mainfrom
metsw24-max:gainmap-checked-pixel-count
Closed

Harden gain-map pixel count arithmetic and dimension validation#3220
metsw24-max wants to merge 1 commit into
AOMediaCodec:mainfrom
metsw24-max:gainmap-checked-pixel-count

Conversation

@metsw24-max
Copy link
Copy Markdown
Contributor

Summary

This PR hardens gain-map dimension handling by introducing centralized checked pixel-count arithmetic and replacing unchecked width * height calculations in gain-map processing paths.

Changes

  • Added avifDimensionsToPixelCount() helper for overflow-safe pixel-count computation
  • Rejected zero-sized and overflow-triggering dimensions
  • Hardened avifDimensionsTooLarge() with explicit zero-dimension validation
  • Replaced unchecked gain-map arithmetic with validated pixel-count usage
  • Added safer allocation-size validation in gain-map computation paths
  • Removed unused return value from avifRWStreamFinishBox()
  • Minor comment/formatting cleanup

Testing

Added regression tests covering:

  • valid pixel-count computation
  • zero dimensions
  • overflow-triggering dimensions
  • invalid dimension handling in avifDimensionsTooLarge()

Validated against existing build/tests and normal gain-map decoding paths.

Comment thread include/avif/internal.h
// marker is the offset of the size field in stream, returned by a previous
// avifRWStreamWriteBox() or avifRWStreamWriteFullBox() call.
AVIF_NODISCARD avifResult avifRWStreamFinishBox(avifRWStream * stream, avifBoxMarker marker);
void avifRWStreamFinishBox(avifRWStream * stream, avifBoxMarker marker);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a merge error.

Comment thread src/gainmap.c
size_t numPixels;
if (!avifDimensionsToPixelCount(width, height, &numPixels)) {
return AVIF_RESULT_INVALID_ARGUMENT;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc: @maryla-uc

metsw24-max: Thank you for your interest in reviewing libavif source code. The code in this file modified by this PR was reviewed at least twice recently. The reason we can multiply width and height (after casting to size_t) with no risk of integer overflow is that a buffer of dimensions width x height has been allocated successfully. Therefore we know width x height must fit in size_t. I am going to decline this PR.

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.

2 participants