Guard against excessively long package names and certificates.

Bug: 303227969
Test: atest CtsBlobStoreTestCases
Flag: EXEMPT security fix
Change-Id: I67ae28b6bc896d57abcb45ef586595bd8aa81346

2 files changed, 35 insertions, 0 deletions

diff --git a/apex/blobstore/framework/java/android/app/blob/BlobStoreManager.java b/apex/blobstore/framework/java/android/app/blob/BlobStoreManager.java
index f6ae56f01758..5b3b876edd3a 100644
--- a/apex/blobstore/framework/java/android/app/blob/BlobStoreManager.java
+++ b/apex/blobstore/framework/java/android/app/blob/BlobStoreManager.java
@@ -32,11 +32,13 @@ import android.os.RemoteCallback;
 import android.os.RemoteException;
 import android.os.UserHandle;
 
+import com.android.internal.util.Preconditions;
 import com.android.internal.util.function.pooled.PooledLambda;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
@@ -153,6 +155,26 @@ public class BlobStoreManager {
     private final Context mContext;
     private final IBlobStoreManager mService;
 
+    // TODO: b/404309424 - Make these constants available using a test-api to avoid hardcoding
+    // them in tests.
+    /**
+     * The maximum allowed length for the package name, provided using
+     * {@link BlobStoreManager.Session#allowPackageAccess(String, byte[])}.
+     *
+     * This is the same limit that is already used for limiting the length of the package names
+     * at android.content.pm.parsing.FrameworkParsingPackageUtils#MAX_FILE_NAME_SIZE.
+     *
+     * @hide
+     */
+    public static final int MAX_PACKAGE_NAME_LENGTH = 223;
+    /**
+     * The maximum allowed length for the certificate, provided using
+     * {@link BlobStoreManager.Session#allowPackageAccess(String, byte[])}.
+     *
+     * @hide
+     */
+    public static final int MAX_CERTIFICATE_LENGTH = 32;
+
     /** @hide */
     public BlobStoreManager(@NonNull Context context, @NonNull IBlobStoreManager service) {
         mContext = context;
@@ -786,6 +808,12 @@ public class BlobStoreManager {
          */
         public void allowPackageAccess(@NonNull String packageName, @NonNull byte[] certificate)
                 throws IOException {
+            Objects.requireNonNull(packageName);
+            Preconditions.checkArgument(packageName.length() <= MAX_PACKAGE_NAME_LENGTH,
+                    "packageName is longer than " + MAX_PACKAGE_NAME_LENGTH + " chars");
+            Objects.requireNonNull(certificate);
+            Preconditions.checkArgument(certificate.length <= MAX_CERTIFICATE_LENGTH,
+                    "certificate is longer than " + MAX_CERTIFICATE_LENGTH + " chars");
             try {
                 mSession.allowPackageAccess(packageName, certificate);
             } catch (ParcelableException e) {
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
index ede29ec168c0..790d4e934317 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
@@ -16,6 +16,8 @@
 package com.android.server.blob;
 
 import static android.app.blob.BlobStoreManager.COMMIT_RESULT_ERROR;
+import static android.app.blob.BlobStoreManager.MAX_CERTIFICATE_LENGTH;
+import static android.app.blob.BlobStoreManager.MAX_PACKAGE_NAME_LENGTH;
 import static android.app.blob.XmlTags.ATTR_CREATION_TIME_MS;
 import static android.app.blob.XmlTags.ATTR_ID;
 import static android.app.blob.XmlTags.ATTR_PACKAGE;
@@ -328,6 +330,11 @@ class BlobStoreSession extends IBlobStoreSession.Stub {
             @NonNull byte[] certificate) {
         assertCallerIsOwner();
         Objects.requireNonNull(packageName, "packageName must not be null");
+        Preconditions.checkArgument(packageName.length() <= MAX_PACKAGE_NAME_LENGTH,
+                "packageName is longer than " + MAX_PACKAGE_NAME_LENGTH + " chars");
+        Objects.requireNonNull(certificate, "certificate must not be null");
+        Preconditions.checkArgument(certificate.length <= MAX_CERTIFICATE_LENGTH,
+                "certificate is longer than " + MAX_CERTIFICATE_LENGTH + " chars");
         synchronized (mSessionLock) {
             if (mState != STATE_OPENED) {
                 throw new IllegalStateException("Not allowed to change access type in state: "