read: prevent integer overflow in image grid validation

[Naveed8951]

Add explicit overflow checks before multiplying tile dimensions by grid rows/columns to avoid wraparound that could bypass validation and lead to malformed image handling.

Reject unsafe grids early with AVIF_RESULT_INVALID_IMAGE_GRID.

[wantehchang]

Since we don't need to store these products in uint32_t variables, it would be simpler and better to just cast tile->image->width and tile->image->height to uint64_ in the four multiplications.

[wantehchang]

You need to first delete the integer overflow checks you added, and then add uint64_t casts to the four multiplications in the original code.

The following patch is the net change:

diff --git a/src/read.c b/src/read.c
index 8f22250d..11f8af07 100644
--- a/src/read.c
+++ b/src/read.c
@@ -1737,14 +1737,14 @@ static avifResult avifDecoderDataAllocateImagePlanes(const avifDecoderData * dat
         //
         // HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1:
         //   The tiled input images shall completely "cover" the reconstructed image grid canvas, ...
-        if (((tile->image->width * grid->columns) < grid->outputWidth) || ((tile->image->height * grid->rows) < grid->outputHeight)) {
+        if ((((uint64_t)tile->image->width * grid->columns) < grid->outputWidth) || (((uint64_t)tile->image->height * grid->rows) < grid->outputHeight)) {
             avifDiagnosticsPrintf(data->diag,
                                   "Grid image tiles do not completely cover the image (HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1)");
             return AVIF_RESULT_INVALID_IMAGE_GRID;
         }
         // Tiles in the rightmost column and bottommost row must overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2.
-        if (((tile->image->width * (grid->columns - 1)) >= grid->outputWidth) ||
-            ((tile->image->height * (grid->rows - 1)) >= grid->outputHeight)) {
+        if ((((uint64_t)tile->image->width * (grid->columns - 1)) >= grid->outputWidth) ||
+            (((uint64_t)tile->image->height * (grid->rows - 1)) >= grid->outputHeight)) {
             avifDiagnosticsPrintf(data->diag,
                                   "Grid image tiles in the rightmost column and bottommost row do not overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2");
             return AVIF_RESULT_INVALID_IMAGE_GRID;

[Naveed8951]

Updated the checks to cast the four multiplications to uint64_t instead of using explicit UINT32_MAX overflow guards. Thanks for the suggestion.

[wantehchang]

You need to first delete the integer overflow checks you added, and then add uint64_t casts to the four multiplications in the original code.

The following patch is the net change:

diff --git a/src/read.c b/src/read.c
index 8f22250d..11f8af07 100644
--- a/src/read.c
+++ b/src/read.c
@@ -1737,14 +1737,14 @@ static avifResult avifDecoderDataAllocateImagePlanes(const avifDecoderData * dat
         //
         // HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1:
         //   The tiled input images shall completely "cover" the reconstructed image grid canvas, ...
-        if (((tile->image->width * grid->columns) < grid->outputWidth) || ((tile->image->height * grid->rows) < grid->outputHeight)) {
+        if ((((uint64_t)tile->image->width * grid->columns) < grid->outputWidth) || (((uint64_t)tile->image->height * grid->rows) < grid->outputHeight)) {
             avifDiagnosticsPrintf(data->diag,
                                   "Grid image tiles do not completely cover the image (HEIF (ISO/IEC 23008-12:2017), Section 6.6.2.3.1)");
             return AVIF_RESULT_INVALID_IMAGE_GRID;
         }
         // Tiles in the rightmost column and bottommost row must overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2.
-        if (((tile->image->width * (grid->columns - 1)) >= grid->outputWidth) ||
-            ((tile->image->height * (grid->rows - 1)) >= grid->outputHeight)) {
+        if ((((uint64_t)tile->image->width * (grid->columns - 1)) >= grid->outputWidth) ||
+            (((uint64_t)tile->image->height * (grid->rows - 1)) >= grid->outputHeight)) {
             avifDiagnosticsPrintf(data->diag,
                                   "Grid image tiles in the rightmost column and bottommost row do not overlap the reconstructed image grid canvas. See MIAF (ISO/IEC 23000-22:2019), Section 7.3.11.4.2, Figure 2");
             return AVIF_RESULT_INVALID_IMAGE_GRID;

[Naveed8951]

Removed the added comment and left only the uint64_t casts as suggested. Thanks.