Fix unchecked integer overflow in encoder UV plane allocation

[jortles]

Summary

• Add pre-multiplication overflow checks to codec_avm.c and codec_svt.c for UV plane size computations, consistent with the existing safe pattern in avifImageAllocatePlanes() (avif.c:435-441)
• Add overflow check in avifImageCopyProperties() before numProperties allocation, consistent with avifImagePushProperty() (avif.c:387)

Details

Three sites compute allocation sizes via unchecked integer multiplication, unlike their parallel functions which validate before multiplying:

codec_avm.c:919 — channelSize * monoUVWidth (uint32_t) and monoUVHeight * monoUVRowBytes (size_t) used for avifAlloc() without overflow checks. The parallel code in avifImageAllocatePlanes() checks width > UINT32_MAX / channelSize and height > PTRDIFF_MAX / fullRowBytes before the same operations.

codec_svt.c:286 — uvWidth * bytesPerPixel (uint32_t) and uvHeight * uvRowBytes (size_t) computed without pre-multiplication validation. The existing post-hoc check (uvSize > UINT32_MAX / 2) cannot detect a wrap that already occurred.

avif.c:237 — numProperties * sizeof(...) passed to avifAlloc() without an overflow guard. avifImagePushProperty() in the same file already checks numProperties < SIZE_MAX / sizeof(avifImageItemProperty) before an identical allocation.

Test plan

• Builds cleanly with cmake --build . --target avif_obj (zero warnings)
• Existing test suite passes (CI)

[y-guyon]

Thank you for your interest in libavif.

[y-guyon]

monoUVWidth is always below UINT32_MAX / channelSize because channelSize is at most 2 and monoUVWidth = (image->width + 1) >> 1.

However I wonder if there should be a check for the image->width + 1 part. Maybe avm_codec_enc_init() fails earlier in case of a huge dimension, but that could change in the future.

[jortles]

Good catch — you're right that monoUVWidth can never exceed UINT32_MAX / channelSize given the current math. I've removed that unreachable check and added a guard for the image->width + 1 overflow instead, which is the actual risk. Same change applied to codec_svt.c.

[y-guyon]

I see these checks were inspired from:

libavif/src/avif.c

Lines 433 to 440 in d8b4e04

	const uint32_t channelSize = avifImageUsesU16(image) ? 2 : 1;
	if (image->width > UINT32_MAX / channelSize) {
	return AVIF_RESULT_INVALID_ARGUMENT;
	}
	const uint32_t fullRowBytes = channelSize * image->width;
	if (image->height > PTRDIFF_MAX / fullRowBytes) {
	return AVIF_RESULT_INVALID_ARGUMENT;
	}

But PTRDIFF_MAX makes little sense in both places in my opinion. It should be SIZE_MAX right?

[jortles]

Agreed — SIZE_MAX is the correct bound here. Changed in both codec_avm.c and codec_svt.c.

[y-guyon]

Maybe image->width should be cast to size_t instead of compared against UINT32_MAX? It does not matter much for such huge dimensions I guess.

[wantehchang]

Yes, we should allow UINT32_MAX.

As Yannis suggested, we can cast image->width to a larger type such as uint64_t. (size_t is 32 bits in 32-bit systems.) We use this approach elsewhere.

Another approach is to replace (image->width + 1) >> 1 with image->width / 2 + image->width % 2, or (since image->width >= 1) 1 + (image->width - 1) / 2, or some other similar trick.