Browse Source

chore: cherry-pick 2 changes from Release-3-M119 (#40648)

* chore: [27-x-y] cherry-pick 2 changes from Release-3-M119

* 6cc0d9aa5b3fb from libavif
* 922fca786b61a from libavif

* chore: remove libavif DEPS patch
Keeley Hammond 1 year ago
parent
commit
5a4c70746b

+ 0 - 1
patches/chromium/.patches

@@ -137,4 +137,3 @@ scale_rects_properly_in_syncgetfirstrectforrange.patch
 fix_restore_original_resize_performance_on_macos.patch
 cherry-pick-3f45b1af5e41.patch
 cherry-pick-e13061c50998.patch
-cherry-pick-6cc0d9aa5b3f.patch

+ 0 - 41
patches/chromium/cherry-pick-6cc0d9aa5b3f.patch

@@ -1,41 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Vignesh Venkatasubramanian <[email protected]>
-Date: Tue, 21 Nov 2023 00:14:19 +0000
-Subject: Roll src/third_party/libavif/src/ d1c26fa..b2d36b1 (2 commits)
-
-https://chromium.googlesource.com/external/github.com/AOMediaCodec/libavif.git/+log/d1c26fa..b2d36b1
-
-$ git log d1c26fa..b2d36b1 --date=short --no-merges --format='%ad %ae %s'
-2023-11-15 [email protected] Remove potential out of bound access to alphaItemIndices
-2023-11-15 [email protected] Do not store potentially invalid pointers
-
-Bug: 1501766, 1501770
-Test: blink_platform_unittests
-Change-Id: I72915b1187ca651e6f47f8d44e946644ebe9fce4
-Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5046038
-Reviewed-by: Dale Curtis <[email protected]>
-Commit-Queue: Vignesh Venkat <[email protected]>
-Reviewed-by: Wan-Teh Chang <[email protected]>
-Cr-Commit-Position: refs/branch-heads/5993@{#1630}
-Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594}
-
-diff --git a/DEPS b/DEPS
-index 79011df08eb07e67c1d4261ca324a9fbce042690..8073700e6367cfcd6910aa80f3a749f3cce6cbfa 100644
---- a/DEPS
-+++ b/DEPS
-@@ -469,7 +469,7 @@ vars = {
-   # Three lines of non-changing comments so that
-   # the commit queue can handle CLs rolling libavif
-   # and whatever else without interference from each other.
--  'libavif_revision': 'd1c26facaf5a8a97919ceee06814d05d10e25622',
-+  'libavif_revision': 'b2d36b1c3bfc806694cd4ff0cb188270823fe6d8',
-   # Three lines of non-changing comments so that
-   # the commit queue can handle CLs rolling libavif
-   # and whatever else without interference from each other.
-diff --git a/third_party/libavif/src b/third_party/libavif/src
-index d1c26facaf5a8a97919ceee06814d05d10e25622..b2d36b1c3bfc806694cd4ff0cb188270823fe6d8 160000
---- a/third_party/libavif/src
-+++ b/third_party/libavif/src
-@@ -1 +1 @@
--Subproject commit d1c26facaf5a8a97919ceee06814d05d10e25622
-+Subproject commit b2d36b1c3bfc806694cd4ff0cb188270823fe6d8

+ 2 - 0
patches/config.json

@@ -11,6 +11,8 @@
 
   "src/electron/patches/node": "src/third_party/electron_node",
 
+  "src/electron/patches/libavif": "src/third_party/libavif/src",
+
   "src/electron/patches/nan": "src/third_party/nan",
 
   "src/electron/patches/perfetto": "src/third_party/perfetto",

+ 2 - 0
patches/libavif/.patches

@@ -0,0 +1,2 @@
+remove_potential_out_of_bound_access_to_alphaitemindices.patch
+do_not_store_potentially_invalid_pointers.patch

+ 68 - 0
patches/libavif/do_not_store_potentially_invalid_pointers.patch

@@ -0,0 +1,68 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Vignesh Venkatasubramanian <[email protected]>
+Date: Wed, 15 Nov 2023 15:22:49 -0800
+Subject: Do not store potentially invalid pointers
+
+Manual cherry-pick of PR #1757 into the chromium-m118 branch.
+
+diff --git a/src/read.c b/src/read.c
+index 53ea5156c04f0f2ce96aaa1762100863309c31e8..d99960e01f1c2dc0ebf7d2cf0dead78bbf8a174b 100644
+--- a/src/read.c
++++ b/src/read.c
+@@ -769,6 +769,8 @@ static void avifMetaDestroy(avifMeta * meta)
+     avifFree(meta);
+ }
+ 
++// CAUTION: This function could potentially resize the meta->items array thereby invalidating all existing pointers that are being
++// stored locally. So if this function is being called, exercise caution in the caller to not use invalid pointers.
+ static avifDecoderItem * avifMetaFindItem(avifMeta * meta, uint32_t itemID)
+ {
+     if (itemID == 0) {
+@@ -3614,17 +3616,20 @@ static avifBool avifDecoderItemIsAlphaAux(avifDecoderItem * item, uint32_t color
+     return auxCProp && isAlphaURN(auxCProp->u.auxC.auxType);
+ }
+ 
+-// Finds the alpha item whose parent item is colorItem and sets it in the alphaItem output parameter. Returns AVIF_RESULT_OK on
+-// success. Note that *alphaItem can be NULL even if the return value is AVIF_RESULT_OK. If the colorItem is a grid and the alpha
+-// item is represented as a set of auxl items to each color tile, then a fake item will be created and *isAlphaItemInInput will be
+-// set to AVIF_FALSE. In this case, the alpha item merely exists to hold the locations of the alpha tile items. The data of this
+-// item need not be read and the pixi property cannot be validated. Otherwise, *isAlphaItemInInput will be set to AVIF_TRUE when
+-// *alphaItem is not NULL.
++// Finds the alpha item whose parent item is *colorItemPtr and sets it in the alphaItem output parameter. Returns AVIF_RESULT_OK
++// on success. Note that *alphaItem can be NULL even if the return value is AVIF_RESULT_OK. If the *colorItemPtr is a grid and the
++// alpha item is represented as a set of auxl items to each color tile, then a fake item will be created and *isAlphaItemInInput
++// will be set to AVIF_FALSE. In this case, the alpha item merely exists to hold the locations of the alpha tile items. The data
++// of this item need not be read and the pixi property cannot be validated. Otherwise, *isAlphaItemInInput will be set to
++// AVIF_TRUE when *alphaItem is not NULL. If the data->meta->items array is resized, then the value in *colorItemPtr could become
++// invalid. This function also resets *colorItemPtr to the right value if an alpha item was found and added to the data->meta->items
++// array.
+ static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data,
+-                                               avifDecoderItem * colorItem,
++                                               avifDecoderItem ** colorItemPtr,
+                                                avifDecoderItem ** alphaItem,
+                                                avifBool * isAlphaItemInInput)
+ {
++    const avifDecoderItem * colorItem = *colorItemPtr;
+     for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) {
+         avifDecoderItem * item = &data->meta->items.item[itemIndex];
+         if (avifDecoderItemShouldBeSkipped(item)) {
+@@ -3700,6 +3705,10 @@ static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data,
+         *isAlphaItemInInput = AVIF_FALSE;
+         return AVIF_RESULT_OUT_OF_MEMORY;
+     }
++    // avifMetaFindItem() could invalidate all existing item pointers. So reset the colorItem pointers.
++    *colorItemPtr = &data->meta->items.item[colorItemIndex];
++    colorItem = *colorItemPtr;
++
+     memcpy((*alphaItem)->type, "grid", 4);
+     (*alphaItem)->width = colorItem->width;
+     (*alphaItem)->height = colorItem->height;
+@@ -3949,7 +3958,7 @@ avifResult avifDecoderReset(avifDecoder * decoder)
+ 
+         avifBool isAlphaItemInInput;
+         avifDecoderItem * alphaItem;
+-        AVIF_CHECKRES(avifDecoderDataFindAlphaItem(data, colorItem, &alphaItem, &isAlphaItemInInput));
++        AVIF_CHECKRES(avifDecoderDataFindAlphaItem(data, &colorItem, &alphaItem, &isAlphaItemInInput));
+         avifCodecType alphaCodecType = AVIF_CODEC_TYPE_UNKNOWN;
+         if (alphaItem) {
+             if (!memcmp(alphaItem->type, "grid", 4)) {

+ 78 - 0
patches/libavif/remove_potential_out_of_bound_access_to_alphaitemindices.patch

@@ -0,0 +1,78 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Vignesh Venkatasubramanian <[email protected]>
+Date: Mon, 13 Nov 2023 19:39:10 -0800
+Subject: Remove potential out of bound access to alphaItemIndices
+
+It is possible to craft a file that has more alpha auxiliary items
+than color items and trigger an out of bound access into
+alphaItemIndices in the for loop.
+
+Fix is to ensure that each color grid item has exactly one alpha
+grid item. Also, ensure that there are exactly the same number of
+color grids as informed in the grid config before trying to
+find the alpha item.
+
+Also, update a diagnostic error message to cover all cases (i.e.)
+there can be more grids than necessary as well.
+
+diff --git a/src/read.c b/src/read.c
+index 3d8ffe28810aece5078b6efa0b19ec3b5d1a03c5..53ea5156c04f0f2ce96aaa1762100863309c31e8 100644
+--- a/src/read.c
++++ b/src/read.c
+@@ -1417,7 +1417,7 @@ static avifBool avifDecoderGenerateImageGridTiles(avifDecoder * decoder, avifIma
+ 
+     if (tilesAvailable != grid->rows * grid->columns) {
+         avifDiagnosticsPrintf(&decoder->diag,
+-                              "Grid image of dimensions %ux%u requires %u tiles, and only %u were found",
++                              "Grid image of dimensions %ux%u requires %u tiles, but %u were found",
+                               grid->columns,
+                               grid->rows,
+                               grid->rows * grid->columns,
+@@ -3659,21 +3659,41 @@ static avifResult avifDecoderDataFindAlphaItem(avifDecoderData * data,
+             maxItemID = item->id;
+         }
+         if (item->dimgForID == colorItem->id) {
++            avifBool seenAlphaForCurrentItem = AVIF_FALSE;
+             for (uint32_t j = 0; j < colorItem->meta->items.count; ++j) {
+                 avifDecoderItem * auxlItem = &colorItem->meta->items.item[j];
+                 if (avifDecoderItemIsAlphaAux(auxlItem, item->id)) {
++                    if (seenAlphaForCurrentItem || auxlItem->dimgForID != 0) {
++                        // One of the following invalid cases:
++                        // * Multiple items are claiming to be the alpha auxiliary of the current item.
++                        // * Alpha auxiliary is dimg for another item.
++                        avifFree(alphaItemIndices);
++                        *isAlphaItemInInput = AVIF_FALSE;
++                        return AVIF_RESULT_INVALID_IMAGE_GRID;
++                    }
+                     alphaItemIndices[alphaItemCount++] = j;
++                    seenAlphaForCurrentItem = AVIF_TRUE;
+                 }
+             }
++            if (!seenAlphaForCurrentItem) {
++                // No alpha auxiliary item was found for the current item. Treat this as an image without alpha.
++                avifFree(alphaItemIndices);
++                *isAlphaItemInInput = AVIF_FALSE;
++                return AVIF_RESULT_OK;
++            }
+         }
+     }
+-    if (alphaItemCount != colorItemCount) {
+-        // Not all the color items had an alpha auxiliary attached to it. Report this case as an image without alpha channel.
+-        avifFree(alphaItemIndices);
+-        *alphaItem = NULL;
+-        *isAlphaItemInInput = AVIF_FALSE;
+-        return AVIF_RESULT_OK;
++    assert(alphaItemCount == colorItemCount);
++
++    int colorItemIndex = -1;
++    for (uint32_t i = 0; i < data->meta->items.count; ++i) {
++        if (colorItem->id == data->meta->items.item[i].id) {
++            colorItemIndex = i;
++            break;
++        }
+     }
++    assert(colorItemIndex >= 0);
++
+     *alphaItem = avifMetaFindItem(colorItem->meta, maxItemID + 1);
+     if (*alphaItem == NULL) {
+         avifFree(alphaItemIndices);