From 753d325302fddfefab4b7003609d912fed5d8fe0 Mon Sep 17 00:00:00 2001
From: Xinyi Ji <[email protected]>
Date: Thu, 23 Jul 2026 19:16:14 -0700
Subject: [PATCH] [CustomizationV2] Fix Rule-of-2 violation and TOCTOU in image customization

This CL addresses two security vulnerabilities in the Android New Tab
Page (NTP) background image customization flow:

1. Rule-of-2 Violation:
   Previously, user-selected background images were decoded directly in
the privileged Browser process using BitmapFactory.decodeStream().
Parsing untrusted image data in native C++ decoders (Skia) within an un-
sandboxed process violates Chromium's Rule of 2.
   This CL refactors image decoding to use the sandboxed Utility process
via data_decoder::DecodeImageIsolated().

2. TOCTOU (Time-of-Check Time-of-Use) Vulnerability:
   Previously, the image URI input stream was opened twice: first to
inspect dimensions (inJustDecodeBounds = true) and second to perform the
actual decode. A malicious ContentProvider could swap the payload
between reads.
   This CL refactors getBitmapFromUriAsync() to read the input stream
exactly once on a background thread into an in-memory byte buffer capped
at 25 MiB (MAX_IMAGE_BYTES), preventing TOCTOU and memory exhaustion
(OOM) attacks.

Key Changes:
- Java (NtpCustomizationUtils.java): Reads URI stream once into a byte
array and passes raw bytes to C++ via JNI.
- C++ JNI Bridge (ntp_customization_utils.cc): Bridges Java byte array
to data_decoder::DecodeImageIsolated(). Once decoded in the sandbox,
DownsampleImageIfNeeded() scales down massive or extreme aspect-ratio
images (e.g. panoramic photos) by powers of 2 using Skia Lanczos3
resampling to prevent Android Canvas/GPU texture rendering crashes,
while retaining full sharpness for slightly oversized images.

Bug: 505967344
Change-Id: Ib03fabbe92a19f402526d703660cbd89d363063f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8111779
Reviewed-by: Xi Han <[email protected]>
Commit-Queue: Xinyi Ji <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1667599}
---
diff --git a/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtils.java b/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtils.java
index c4975703..3f7a8ec 100644
--- a/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtils.java
+++ b/chrome/browser/ntp_customization/java/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtils.java
@@ -41,6 +41,7 @@
 import android.app.Activity;
 import android.content.Context;
 import android.content.Intent;
+import android.content.res.AssetFileDescriptor;
 import android.content.res.ColorStateList;
 import android.content.res.Configuration;
 import android.content.res.Resources;
@@ -70,6 +71,10 @@
 import com.google.android.material.color.DynamicColors;
 import com.google.android.material.color.DynamicColorsOptions;
 
+import org.jni_zero.JNINamespace;
+import org.jni_zero.JniType;
+import org.jni_zero.NativeMethods;
+
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.IntentUtils;
@@ -109,6 +114,7 @@
 import org.chromium.ui.util.ColorUtils;
 import org.chromium.url.GURL;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -119,6 +125,7 @@
 
 /** Utility class of the NTP customization. */
 @NullMarked
+@JNINamespace("ntp_customization")
 public class NtpCustomizationUtils {
 
     // LINT.IfChange(NtpBackgroundType)
@@ -161,15 +168,23 @@
     }
 
     private static class ImageLoadResult {
-        public final @Nullable Bitmap bitmap;
+        public final byte @Nullable [] data;
         public final String fileIdHash;
 
-        ImageLoadResult(@Nullable Bitmap bitmap, String fileIdHash) {
-            this.bitmap = bitmap;
+        ImageLoadResult(byte @Nullable [] data, String fileIdHash) {
+            this.data = data;
             this.fileIdHash = fileIdHash;
         }
     }
 
+    // Maximum allowed byte size for user-uploaded custom background images (25 MiB) to prevent
+    // memory exhaustion.
+    private static final int MAX_IMAGE_BYTES = 25 * 1024 * 1024;
+    // Initial in-memory buffer capacity when reading background image stream (1 MiB).
+    private static final int INITIAL_BUFFER_CAPACITY_BYTES = 1024 * 1024;
+    // Chunk size used to read the background image stream (8 KiB).
+    private static final int READ_BUFFER_SIZE_BYTES = 8192;
+
     public static final String NTP_UPLOAD_IMAGES_DIR = "upload_images";
     public static final String NTP_THEME_COLLECTION_IMAGES_DIR = "theme_collection_images";
 
@@ -179,7 +194,6 @@
     static final String NTP_BACKGROUND_IMAGE_FILE_FOR_DAILY_REFRESH =
             "ntp_background_image_for_daily_refresh";
 
-    private static final int MAX_IMAGE_SIZE = 2556;
     private static final int IMAGE_SIZE_FOR_EXTRACTING_COLOR = 100;
     private static final String TAG = "NtpCustomization";
     private static final String DELIMITER = "|";
@@ -1770,25 +1784,41 @@
             @Override
             protected ImageLoadResult doInBackground() {
                 String fileIdHash = NtpBackgroundDataUtils.getMetadataFingerprint(context, uri);
-                try {
-                    // 1. Decode with inJustDecodeBounds=true to check dimensions
-                    BitmapFactory.Options options = new BitmapFactory.Options();
-                    options.inJustDecodeBounds = true;
-                    try (var inputStream = context.getContentResolver().openInputStream(uri)) {
-                        BitmapFactory.decodeStream(inputStream, null, options);
-                    }
-
-                    // 2. Calculate inSampleSize
-                    options.inSampleSize =
-                            calculateInSampleSize(options, MAX_IMAGE_SIZE, MAX_IMAGE_SIZE);
-
-                    // 3. Decode bitmap with inSampleSize set
-                    options.inJustDecodeBounds = false;
-                    try (var inputStream = context.getContentResolver().openInputStream(uri)) {
-                        return new ImageLoadResult(
-                                BitmapFactory.decodeStream(inputStream, null, options), fileIdHash);
+                try (AssetFileDescriptor afd =
+                        context.getContentResolver().openAssetFileDescriptor(uri, "r")) {
+                    if (afd != null) {
+                        long fileLength = afd.getLength();
+                        if (fileLength != AssetFileDescriptor.UNKNOWN_LENGTH
+                                && fileLength > MAX_IMAGE_BYTES) {
+                            Log.w(TAG, "Image exceeds maximum allowed size cap of 25 MiB.");
+                            return new ImageLoadResult(null, fileIdHash);
+                        }
                     }
                 } catch (Exception e) {
+                    // Ignore and proceed to stream read, as some content providers might not
+                    // support openAssetFileDescriptor.
+                }
+
+                try (var inputStream = context.getContentResolver().openInputStream(uri)) {
+                    if (inputStream == null) {
+                        return new ImageLoadResult(null, fileIdHash);
+                    }
+
+                    ByteArrayOutputStream buffer =
+                            new ByteArrayOutputStream(INITIAL_BUFFER_CAPACITY_BYTES);
+                    byte[] data = new byte[READ_BUFFER_SIZE_BYTES];
+                    int nRead;
+                    int totalRead = 0;
+                    while ((nRead = inputStream.read(data, 0, data.length)) != -1) {
+                        totalRead += nRead;
+                        if (totalRead > MAX_IMAGE_BYTES) {
+                            Log.w(TAG, "Image exceeds maximum allowed size cap of 25 MiB.");
+                            return new ImageLoadResult(null, fileIdHash);
+                        }
+                        buffer.write(data, 0, nRead);
+                    }
+                    return new ImageLoadResult(buffer.toByteArray(), fileIdHash);
+                } catch (Exception | OutOfMemoryError e) {
                     Log.e(TAG, "Error reading bitmap from URI", e);
                     return new ImageLoadResult(null, fileIdHash);
                 }
@@ -1796,32 +1826,19 @@
 
             @Override
             protected void onPostExecute(ImageLoadResult result) {
-                callback.onImageLoaded(result.bitmap, result.fileIdHash);
+                byte @Nullable [] data = result.data;
+                if (data == null) {
+                    callback.onImageLoaded(null, result.fileIdHash);
+                    return;
+                }
+
+                NtpCustomizationUtilsJni.get()
+                        .decodeImage(
+                                data, bitmap -> callback.onImageLoaded(bitmap, result.fileIdHash));
             }
         }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
     }
 
-    @VisibleForTesting
-    static int calculateInSampleSize(BitmapFactory.Options options, int reqWidth, int reqHeight) {
-        // Raw height and width of image
-        final int height = options.outHeight;
-        final int width = options.outWidth;
-        int inSampleSize = 1;
-
-        if (height > reqHeight || width > reqWidth) {
-            final int halfHeight = height / 2;
-            final int halfWidth = width / 2;
-
-            // Calculate the largest inSampleSize value that is a power of 2 and keeps height or
-            // width larger than the requested height and width.
-            while ((halfHeight / inSampleSize) >= reqHeight
-                    || (halfWidth / inSampleSize) >= reqWidth) {
-                inSampleSize *= 2;
-            }
-        }
-        return inSampleSize;
-    }
-
     /** Returns whether the theme tip bottom sheet has been shown before. */
     public static void resetThemeTipBottomSheetShownTimestampFromSharedPreferenceForTesting() {
         SharedPreferencesManager prefsManager = ChromeSharedPreferences.getInstance();
@@ -1853,4 +1870,9 @@
     public static File createThemeCollectionImageFileInDirForTesting(String fileName) {
         return createThemeImageFileInDir(fileName, NTP_THEME_COLLECTION_IMAGES_DIR);
     }
+
+    @NativeMethods
+    public interface Natives {
+        void decodeImage(@JniType("std::vector<uint8_t>") byte[] data, Callback<Bitmap> callback);
+    }
 }
diff --git a/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtilsUnitTest.java b/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtilsUnitTest.java
index d65d74a4..ece9aa1 100644
--- a/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtilsUnitTest.java
+++ b/chrome/browser/ntp_customization/junit/src/org/chromium/chrome/browser/ntp_customization/NtpCustomizationUtilsUnitTest.java
@@ -11,9 +11,12 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeFalse;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -41,10 +44,10 @@
 
 import android.content.ContentResolver;
 import android.content.Context;
+import android.content.res.AssetFileDescriptor;
 import android.content.res.ColorStateList;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
-import android.graphics.BitmapFactory;
 import android.graphics.Color;
 import android.graphics.Matrix;
 import android.graphics.Point;
@@ -109,20 +112,29 @@
 import org.chromium.url.JUnitTestGURLs;
 
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 
 /** Unit tests for {@link NtpCustomizationUtils} */
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE, sdk = Build.VERSION_CODES.R)
 public class NtpCustomizationUtilsUnitTest {
+    private static final String TEST_FILE_NAME = "test_file.png";
+    private static final String LARGE_FILE_NAME = "large_file.png";
+    private static final String ERROR_FILE_NAME = "error_file.png";
+    private static final String INVALID_FILE_NAME = "invalid_file.png";
+    private static final String NULL_FILE_NAME = "null_file.png";
+    private static final String FILE_ID_HASH_SUFFIX = "_-1";
+    private static final String STREAM_ERROR_MESSAGE = "Stream error";
+
     @Rule public MockitoRule mMockitoRule = MockitoJUnit.rule();
 
     @Mock private Tab mTab;
     @Mock private Drawable mDrawable;
     @Mock private NtpCustomizationConfigManager mConfigManager;
     @Mock private WindowAndroid mWindowAndroid;
+    @Mock private NtpCustomizationUtils.Natives mMockJni;
 
     private Context mContext;
     private Resources mResources;
@@ -136,6 +148,7 @@
                         R.style.Theme_BrowserUI_DayNight);
         mResources = mContext.getResources();
         NtpCustomizationConfigManager.setInstanceForTesting(mConfigManager);
+        NtpCustomizationUtilsJni.setInstanceForTesting(mMockJni);
 
         mEdgeToEdgeStateProvider = NtpCustomizationTestHelper.setupEdgeToEdge(mWindowAndroid);
     }
@@ -1752,65 +1765,145 @@
     }
 
     @Test
-    public void testGetBitmapFromUriAsync() throws IOException {
-        int width = 100;
-        int height = 100;
-        Bitmap bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888);
-        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-        bitmap.compress(Bitmap.CompressFormat.PNG, 100, outputStream);
-        byte[] bitmapBytes = outputStream.toByteArray();
-
+    public void testGetBitmapFromUriAsync_SingleReadSuccess() throws IOException {
+        String fileName = TEST_FILE_NAME;
+        String expectedFileIdHash = fileName + FILE_ID_HASH_SUFFIX;
+        byte[] bitmapBytes = new byte[] {1, 2, 3, 4, 5};
         Uri uri = mock(Uri.class);
         ContentResolver contentResolver = mock(ContentResolver.class);
-        // We need to spy or mock Context because mContext is a ContextThemeWrapper which wraps
-        // ApplicationContext.
-        Context context = spy(mContext);
-        when(context.getContentResolver()).thenReturn(contentResolver);
-        when(contentResolver.openInputStream(uri))
-                .thenAnswer(invocation -> new ByteArrayInputStream(bitmapBytes));
-
-        when(uri.getLastPathSegment()).thenReturn("test_file.png");
+        setupMockInputStream(contentResolver, uri, bitmapBytes);
+        Bitmap mockBitmap = setupMockJniDecodeSuccess();
 
         NtpCustomizationUtils.OnImageLoadedCallback callback =
-                mock(NtpCustomizationUtils.OnImageLoadedCallback.class);
-        NtpCustomizationUtils.getBitmapFromUriAsync(context, uri, callback);
-        RobolectricUtil.runAllBackgroundAndUi();
+                loadBitmapFromUriForTesting(uri, fileName, contentResolver);
 
-        ArgumentCaptor<Bitmap> captor = ArgumentCaptor.forClass(Bitmap.class);
-        ArgumentCaptor<String> fileIdHashCaptor = ArgumentCaptor.forClass(String.class);
-        verify(callback).onImageLoaded(captor.capture(), fileIdHashCaptor.capture());
-        Bitmap result = captor.getValue();
-        assertEquals("test_file.png_-1", fileIdHashCaptor.getValue());
-        assertNotNull("The file reading flow should successfully load the bitmap.", result);
-        assertEquals(
-                "The file reading flow should preserve width for small images.",
-                width,
-                result.getWidth());
-        assertEquals(
-                "The file reading flow should preserve height for small images.",
-                height,
-                result.getHeight());
+        verify(contentResolver).openInputStream(uri);
+        verify(mMockJni).decodeImage(eq(bitmapBytes), any());
+        verify(callback).onImageLoaded(eq(mockBitmap), eq(expectedFileIdHash));
     }
 
     @Test
-    public void testCalculateInSampleSize() {
-        BitmapFactory.Options options = new BitmapFactory.Options();
-        options.outWidth = 500;
-        options.outHeight = 500;
+    public void testGetBitmapFromUriAsync_ExceedsMaxSizeCap() throws IOException {
+        String fileName = LARGE_FILE_NAME;
+        String expectedFileIdHash = fileName + FILE_ID_HASH_SUFFIX;
+        byte[] largeBytes = new byte[25 * 1024 * 1024 + 100];
+        Uri uri = mock(Uri.class);
+        ContentResolver contentResolver = mock(ContentResolver.class);
+        setupMockInputStream(contentResolver, uri, largeBytes);
 
-        // Test no downsampling needed.
-        assertEquals(
-                1,
-                NtpCustomizationUtils.calculateInSampleSize(
-                        options, /* reqWidth= */ 500, /* reqHeight= */ 500));
+        NtpCustomizationUtils.OnImageLoadedCallback callback =
+                loadBitmapFromUriForTesting(uri, fileName, contentResolver);
 
-        // Test downsampling.
-        options.outWidth = 2000;
-        options.outHeight = 2000;
-        assertEquals(
-                4,
-                NtpCustomizationUtils.calculateInSampleSize(
-                        options, /* reqWidth= */ 500, /* reqHeight= */ 500));
+        verify(contentResolver).openInputStream(uri);
+        verify(mMockJni, never()).decodeImage(any(), any());
+        verify(callback).onImageLoaded(isNull(), eq(expectedFileIdHash));
+    }
+
+    @Test
+    public void testGetBitmapFromUriAsync_StreamIOException() throws IOException {
+        String fileName = ERROR_FILE_NAME;
+        String expectedFileIdHash = fileName + FILE_ID_HASH_SUFFIX;
+        String errorMessage = STREAM_ERROR_MESSAGE;
+        Uri uri = mock(Uri.class);
+        ContentResolver contentResolver = mock(ContentResolver.class);
+        when(contentResolver.openInputStream(uri))
+                .thenThrow(new FileNotFoundException(errorMessage));
+
+        NtpCustomizationUtils.OnImageLoadedCallback callback =
+                loadBitmapFromUriForTesting(uri, fileName, contentResolver);
+
+        verify(mMockJni, never()).decodeImage(any(), any());
+        verify(callback).onImageLoaded(isNull(), eq(expectedFileIdHash));
+    }
+
... (file diff truncated)

diff --git a/chrome/browser/ntp_customization/ntp_customization_utils_unittest.cc b/chrome/browser/ntp_customization/ntp_customization_utils_unittest.cc
new file mode 100644
index 0000000..c76a48b
--- /dev/null
+++ b/chrome/browser/ntp_customization/ntp_customization_utils_unittest.cc
@@ -0,0 +1,59 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
+
+namespace ntp_customization {
+
+SkBitmap DownsampleImageIfNeeded(const SkBitmap& bitmap, int max_dimension);
+
+constexpr int kMaxDimension = 2556;
+
+static SkBitmap CreateTestBitmap(int width, int height) {
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(width, height);
+  bitmap.eraseColor(SK_ColorBLUE);
+  return bitmap;
+}
+
+TEST(NtpCustomizationUtilsTest, DownsampleImageIfNeeded_SmallImage) {
+  SkBitmap small_bitmap = CreateTestBitmap(500, 500);
+  SkBitmap result = DownsampleImageIfNeeded(small_bitmap, kMaxDimension);
+
+  EXPECT_EQ(result.width(), 500);
+  EXPECT_EQ(result.height(), 500);
+}
+
+TEST(NtpCustomizationUtilsTest,
+     DownsampleImageIfNeeded_SlightlyExceedsMaxDimension) {
+  // 3000x2000 exceeds 2556, but its halved dimension (1500) is < 2556.
+  // Downsampling is not triggered, preserving full resolution sharpness.
+  SkBitmap bitmap = CreateTestBitmap(3000, 2000);
+  SkBitmap result = DownsampleImageIfNeeded(bitmap, kMaxDimension);
+
+  EXPECT_EQ(result.width(), 3000);
+  EXPECT_EQ(result.height(), 2000);
+}
+
+TEST(NtpCustomizationUtilsTest, DownsampleImageIfNeeded_MassiveImage) {
+  // 6000x4000 has halved width 3000 >= 2556, so sample_size becomes 2.
+  SkBitmap large_bitmap = CreateTestBitmap(6000, 4000);
+  SkBitmap result = DownsampleImageIfNeeded(large_bitmap, kMaxDimension);
+
+  EXPECT_EQ(result.width(), 3000);
+  EXPECT_EQ(result.height(), 2000);
+}
+
+TEST(NtpCustomizationUtilsTest, DownsampleImageIfNeeded_PanoramicImage) {
+  // 8000x1000 has halved width 4000 >= 2556, so sample_size becomes 2.
+  SkBitmap panoramic_bitmap = CreateTestBitmap(8000, 1000);
+  SkBitmap result = DownsampleImageIfNeeded(panoramic_bitmap, kMaxDimension);
+
+  EXPECT_EQ(result.width(), 4000);
+  EXPECT_EQ(result.height(), 500);
+}
+
+}  // namespace ntp_customization