commit 01aac2c5d357e1a12a040645cb66467b8776605d
author Peter Li <lpeter@google.com> Mon Jun 03 09:47:04 2024 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Mon Jun 03 09:47:04 2024 +0000
tree b6a2dcc4913c9dc08b933c041ec3d2e76afa5da7
parent adbb6b6d20d154426c0169ee2688110c988547ff
parent 5670372855415aaf3502da0289cff7a74de0bda2

Merge "Add the protection to avoid data overflow in BinaryXmlSerializer.java" into sc-dev am: 039660c4b0 am: 206430020b am: 0b21dd3c80 am: 5670372855

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/27416928

Change-Id: I99fd45ce2bd3f9d5cc4d00c33b204fb1e46bf5f0
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

core/java/com/android/internal/util/BinaryXmlSerializer.java

diff --git a/core/java/com/android/internal/util/BinaryXmlSerializer.java b/core/java/com/android/internal/util/BinaryXmlSerializer.java
index f0ca1edb..ae969a8 100644
--- a/core/java/com/android/internal/util/BinaryXmlSerializer.java
+++ b/core/java/com/android/internal/util/BinaryXmlSerializer.java
@@ -97,6 +97,8 @@
      */
     private static final int BUFFER_SIZE = 32_768;
 
+    private static final int MAX_UNSIGNED_SHORT = 65_535;
+
     private FastDataOutput mOut;
 
     /**
@@ -226,6 +228,10 @@
         if (namespace != null && !namespace.isEmpty()) throw illegalNamespace();
         mOut.writeByte(ATTRIBUTE | TYPE_BYTES_HEX);
         mOut.writeInternedUTF(name);
+        if (value.length > MAX_UNSIGNED_SHORT) {
+            throw new IOException("attributeBytesHex: input size (" + value.length
+                    + ") exceeds maximum allowed size (" + MAX_UNSIGNED_SHORT + ")");
+        }
         mOut.writeShort(value.length);
         mOut.write(value);
         return this;
@@ -237,6 +243,10 @@
         if (namespace != null && !namespace.isEmpty()) throw illegalNamespace();
         mOut.writeByte(ATTRIBUTE | TYPE_BYTES_BASE64);
         mOut.writeInternedUTF(name);
+        if (value.length > MAX_UNSIGNED_SHORT) {
+            throw new IOException("attributeBytesBase64: input size (" + value.length
+                    + ") exceeds maximum allowed size (" + MAX_UNSIGNED_SHORT + ")");
+        }
         mOut.writeShort(value.length);
         mOut.write(value);
         return this;

core/tests/coretests/src/android/util/BinaryXmlTest.java

diff --git a/core/tests/coretests/src/android/util/BinaryXmlTest.java b/core/tests/coretests/src/android/util/BinaryXmlTest.java
index fd625dce..b369868 100644
--- a/core/tests/coretests/src/android/util/BinaryXmlTest.java
+++ b/core/tests/coretests/src/android/util/BinaryXmlTest.java
@@ -24,6 +24,8 @@
 import static android.util.XmlTest.doVerifyWrite;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.fail;
 import static org.xmlpull.v1.XmlPullParser.START_TAG;
 
 import android.os.PersistableBundle;
@@ -38,12 +40,15 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 
 @RunWith(AndroidJUnit4.class)
 public class BinaryXmlTest {
+    private static final int MAX_UNSIGNED_SHORT = 65_535;
+
     /**
      * Verify that we can write and read large numbers of interned
      * {@link String} values.
@@ -167,4 +172,49 @@
             }
         }
     }
+
+    @Test
+    public void testAttributeBytes_BinaryDataOverflow() throws Exception {
+        final TypedXmlSerializer out = Xml.newBinarySerializer();
+        final ByteArrayOutputStream os = new ByteArrayOutputStream();
+        out.setOutput(os, StandardCharsets.UTF_8.name());
+
+        final byte[] testBytes = new byte[MAX_UNSIGNED_SHORT + 1];
+        assertThrows(IOException.class,
+                () -> out.attributeBytesHex(/* namespace */ null, /* name */ "attributeBytesHex",
+                        testBytes));
+
+        assertThrows(IOException.class,
+                () -> out.attributeBytesBase64(/* namespace */ null, /* name */
+                        "attributeBytesBase64", testBytes));
+    }
+
+    @Test
+    public void testAttributeBytesHex_MaximumBinaryData() throws Exception {
+        final TypedXmlSerializer out = Xml.newBinarySerializer();
+        final ByteArrayOutputStream os = new ByteArrayOutputStream();
+        out.setOutput(os, StandardCharsets.UTF_8.name());
+
+        final byte[] testBytes = new byte[MAX_UNSIGNED_SHORT];
+        try {
+            out.attributeBytesHex(/* namespace */ null, /* name */ "attributeBytesHex", testBytes);
+        } catch (Exception e) {
+            fail("testAttributeBytesHex fails with exception: " + e.toString());
+        }
+    }
+
+    @Test
+    public void testAttributeBytesBase64_MaximumBinaryData() throws Exception {
+        final TypedXmlSerializer out = Xml.newBinarySerializer();
+        final ByteArrayOutputStream os = new ByteArrayOutputStream();
+        out.setOutput(os, StandardCharsets.UTF_8.name());
+
+        final byte[] testBytes = new byte[MAX_UNSIGNED_SHORT];
+        try {
+            out.attributeBytesBase64(/* namespace */ null, /* name */ "attributeBytesBase64",
+                    testBytes);
+        } catch (Exception e) {
+            fail("testAttributeBytesBase64 fails with exception: " + e.toString());
+        }
+    }
 }