From 9b82a24f0e7a4f128782269be9e97fea64b393f8 Mon Sep 17 00:00:00 2001
From: Paulo Casanova <pasc@google.com>
Date: Thu, 2 Nov 2017 10:53:45 -0700
Subject: Fix buffer underflow bug in apkzlib.

If the extra fields has a header with data size that exceeds the available
buffer, ExtraField would throw a buffer underflow instead of IOException
and this would not be caught in ZFile's update.

Test: included
Bug: 68763077
Change-Id: I34db018526c8cd3a2bc3ebf6fb97604ed941ddfa
---
 .../java/com/android/apkzlib/zip/ExtraField.java   | 10 +++++++++
 .../com/android/apkzlib/zip/ExtraFieldTest.java    | 26 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)

(limited to 'src')

diff --git a/src/main/java/com/android/apkzlib/zip/ExtraField.java b/src/main/java/com/android/apkzlib/zip/ExtraField.java
index 90c6fae..d70fa7f 100644
--- a/src/main/java/com/android/apkzlib/zip/ExtraField.java
+++ b/src/main/java/com/android/apkzlib/zip/ExtraField.java
@@ -158,6 +158,16 @@ public class ExtraField {
             }
 
             byte[] data = new byte[dataSize];
+            if (buffer.remaining() < dataSize) {
+                throw new IOException(
+                        "Invalid data size for extra field segment with header ID "
+                                + headerId
+                                + ": "
+                                + dataSize
+                                + " (only "
+                                + buffer.remaining()
+                                + " bytes are available)");
+            }
             buffer.get(data);
 
             SegmentFactory factory = identifySegmentFactory(headerId);
diff --git a/src/test/java/com/android/apkzlib/zip/ExtraFieldTest.java b/src/test/java/com/android/apkzlib/zip/ExtraFieldTest.java
index d80ccc4..2371849 100644
--- a/src/test/java/com/android/apkzlib/zip/ExtraFieldTest.java
+++ b/src/test/java/com/android/apkzlib/zip/ExtraFieldTest.java
@@ -19,6 +19,7 @@ package com.android.apkzlib.zip;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 
@@ -332,4 +333,29 @@ public class ExtraFieldTest {
             assertArrayEquals(new byte[] { 0x54, 0x76, 0x04, 0x00, 2, 4, 2, 4 }, sData);
         }
     }
+
+    @Test
+    public void parseInvalidExtraFieldWithInvalidHeader() throws Exception {
+        byte[] raw = new byte[1];
+        ExtraField ef = new ExtraField(raw);
+        try {
+            ef.getSegments();
+            fail();
+        } catch (IOException e) {
+            // Expected.
+        }
+    }
+
+    @Test
+    public void parseInvalidExtraFieldWithInsufficientData() throws Exception {
+        // Remember: 0x05, 0x00 = 5 in little endian!
+        byte[] raw = new byte[] { /* Header */ 0x01, 0x02, /* Size */ 0x05, 0x00, /* Data */ 0x01 };
+        ExtraField ef = new ExtraField(raw);
+        try {
+            ef.getSegments();
+            fail();
+        } catch (IOException e) {
+            // Expected.
+        }
+    }
 }
-- 
cgit v1.2.3