Skip to content

gainmap: prevent integer overflow in plane allocation #3049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rootvector2 wants to merge 1 commit into from
Closed

gainmap: prevent integer overflow in plane allocation #3049

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/gainmap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,11 +558,24 @@ avifResult avifRGBImageComputeGainMap(const avifRGBImage * baseRgbImage,

avifResult res = AVIF_RESULT_OK;
// --- After this point, the function should exit with 'goto cleanup' to free allocated resources.
// Overflow protection: 'width * height * sizeof(float)' uses signed int
// multiplication which is undefined behavior on overflow in C. Compute the
// allocation size in size_t with explicit overflow checks instead.
Copy link
Collaborator

@wantehchang wantehchang Feb 21, 2026

Choose a reason for hiding this comment

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

[All of my review comments are for future reference.]

Omit this kind of comment that explains an overflow check in detail. There are a lot of such overflow checks in libavif. We cannot afford to explain all of them in detail. libavif maintainers are expected to be familiar with them.

if (baseRgbImage->width > SIZE_MAX / sizeof(float)) {
res = AVIF_RESULT_INVALID_ARGUMENT;
goto cleanup;
}
const size_t gainMapPlaneRowBytes = (size_t)baseRgbImage->width * sizeof(float);
if (gainMapPlaneRowBytes != 0 && baseRgbImage->height > SIZE_MAX / gainMapPlaneRowBytes) {
res = AVIF_RESULT_INVALID_ARGUMENT;
goto cleanup;
}
Copy link
Collaborator

@wantehchang wantehchang Feb 21, 2026

Choose a reason for hiding this comment

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

If the width and height are for a buffer that have been allocated successfully, it is safe to multiply width and height in the size_t type. We can take advantage of this fact here. So for the problematic expression width * height * sizeof(float), we only need one overflow check:

    const size_t numPixels = (size_t)width * height;
    if (numPixels > SIZE_MAX / sizeof(float)) {
        res = AVIF_RESULT_INVALID_ARGUMENT;
        goto cleanup;
    }
    const size_t gainMapPlaneSize = numPixels * sizeof(float);

const size_t gainMapPlaneSize = gainMapPlaneRowBytes * baseRgbImage->height;

const avifBool singleChannel = (gainMap->image->yuvFormat == AVIF_PIXEL_FORMAT_YUV400);
const int numGainMapChannels = singleChannel ? 1 : 3;
for (int c = 0; c < numGainMapChannels; ++c) {
gainMapF[c] = avifAlloc(width * height * sizeof(float));
gainMapF[c] = avifAlloc(gainMapPlaneSize);
if (gainMapF[c] == NULL) {
res = AVIF_RESULT_OUT_OF_MEMORY;
goto cleanup;
Expand Down
Loading