From 4e8469cb456ad25d4dd448079bb46a938ad6e306 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Wed, 11 Sep 2024 17:19:12 -0700 Subject: [PATCH] Allocate correct-sized array when parsing packed fixed-width primitives But check that the given byte size is within the bounds of the byte[] first, otherwise we might get OutOfMemoryErrors on bitflipped input that tries to allocate huge arrays. Add some tests that would have otherwise OutOfMemoryError'd in order to have some confidence that we won't OOM. PiperOrigin-RevId: 673597245 --- .../com/google/protobuf/ArrayDecoders.java | 28 ++++++++++++--- .../com/google/protobuf/BooleanArrayList.java | 26 +++++++++++--- .../com/google/protobuf/DoubleArrayList.java | 26 +++++++++++--- .../com/google/protobuf/FloatArrayList.java | 26 +++++++++++--- .../com/google/protobuf/IntArrayList.java | 26 +++++++++++--- .../com/google/protobuf/LongArrayList.java | 26 +++++++++++--- .../google/protobuf/ArrayDecodersTest.java | 36 +++++++++++++++++++ 7 files changed, 170 insertions(+), 24 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/ArrayDecoders.java b/java/core/src/main/java/com/google/protobuf/ArrayDecoders.java index 889b7bcfa3e6..c3d633d0a63a 100644 --- a/java/core/src/main/java/com/google/protobuf/ArrayDecoders.java +++ b/java/core/src/main/java/com/google/protobuf/ArrayDecoders.java @@ -459,7 +459,12 @@ static int decodePackedFixed32List( throws InvalidProtocolBufferException { final IntArrayList output = (IntArrayList) list; position = decodeVarint32(data, position, registers); - final int fieldLimit = position + registers.int1; + final int packedDataByteSize = registers.int1; + final int fieldLimit = position + packedDataByteSize; + if (fieldLimit > data.length) { + throw InvalidProtocolBufferException.truncatedMessage(); + } + output.ensureCapacity(output.size() + packedDataByteSize / 4); while (position < fieldLimit) { output.addInt(decodeFixed32(data, position)); position += 4; @@ -476,7 +481,12 @@ static int decodePackedFixed64List( throws InvalidProtocolBufferException { final LongArrayList output = (LongArrayList) list; position = decodeVarint32(data, position, registers); - final int fieldLimit = position + registers.int1; + final int packedDataByteSize = registers.int1; + final int fieldLimit = position + packedDataByteSize; + if (fieldLimit > data.length) { + throw InvalidProtocolBufferException.truncatedMessage(); + } + output.ensureCapacity(output.size() + packedDataByteSize / 8); while (position < fieldLimit) { output.addLong(decodeFixed64(data, position)); position += 8; @@ -493,7 +503,12 @@ static int decodePackedFloatList( throws InvalidProtocolBufferException { final FloatArrayList output = (FloatArrayList) list; position = decodeVarint32(data, position, registers); - final int fieldLimit = position + registers.int1; + final int packedDataByteSize = registers.int1; + final int fieldLimit = position + packedDataByteSize; + if (fieldLimit > data.length) { + throw InvalidProtocolBufferException.truncatedMessage(); + } + output.ensureCapacity(output.size() + packedDataByteSize / 4); while (position < fieldLimit) { output.addFloat(decodeFloat(data, position)); position += 4; @@ -510,7 +525,12 @@ static int decodePackedDoubleList( throws InvalidProtocolBufferException { final DoubleArrayList output = (DoubleArrayList) list; position = decodeVarint32(data, position, registers); - final int fieldLimit = position + registers.int1; + final int packedDataByteSize = registers.int1; + final int fieldLimit = position + packedDataByteSize; + if (fieldLimit > data.length) { + throw InvalidProtocolBufferException.truncatedMessage(); + } + output.ensureCapacity(output.size() + packedDataByteSize / 8); while (position < fieldLimit) { output.addDouble(decodeDouble(data, position)); position += 8; diff --git a/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java b/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java index 2c78b0d2910c..353486db8311 100644 --- a/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java @@ -170,8 +170,7 @@ public void add(int index, Boolean element) { public void addBoolean(boolean element) { ensureIsMutable(); if (size == array.length) { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); boolean[] newArray = new boolean[length]; System.arraycopy(array, 0, newArray, 0, size); @@ -192,8 +191,7 @@ private void addBoolean(int index, boolean element) { // Shift everything over to make room System.arraycopy(array, index, array, index + 1, size - index); } else { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); boolean[] newArray = new boolean[length]; // Copy the first part directly @@ -255,6 +253,26 @@ public Boolean remove(int index) { return value; } + /** Ensures the backing array can fit at least minCapacity elements. */ + void ensureCapacity(int minCapacity) { + if (minCapacity <= array.length) { + return; + } + // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to + // exactly the requested capacity, but must exponentially grow instead. This is similar + // behaviour to ArrayList. + int n = array.length; + while (n < minCapacity) { + n = growSize(n); + } + array = Arrays.copyOf(array, n); + } + + private static int growSize(int previousSize) { + // Resize to 1.5x the size + return ((previousSize * 3) / 2) + 1; + } + /** * Ensures that the provided {@code index} is within the range of {@code [0, size]}. Throws an * {@link IndexOutOfBoundsException} if it is not. diff --git a/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java b/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java index 32b6b6781c9e..1c7b009dc7d2 100644 --- a/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java @@ -170,8 +170,7 @@ public void add(int index, Double element) { public void addDouble(double element) { ensureIsMutable(); if (size == array.length) { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); double[] newArray = new double[length]; System.arraycopy(array, 0, newArray, 0, size); @@ -192,8 +191,7 @@ private void addDouble(int index, double element) { // Shift everything over to make room System.arraycopy(array, index, array, index + 1, size - index); } else { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); double[] newArray = new double[length]; // Copy the first part directly @@ -255,6 +253,26 @@ public Double remove(int index) { return value; } + /** Ensures the backing array can fit at least minCapacity elements. */ + void ensureCapacity(int minCapacity) { + if (minCapacity <= array.length) { + return; + } + // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to + // exactly the requested capacity, but must exponentially grow instead. This is similar + // behaviour to ArrayList. + int n = array.length; + while (n < minCapacity) { + n = growSize(n); + } + array = Arrays.copyOf(array, n); + } + + private static int growSize(int previousSize) { + // Resize to 1.5x the size + return ((previousSize * 3) / 2) + 1; + } + /** * Ensures that the provided {@code index} is within the range of {@code [0, size]}. Throws an * {@link IndexOutOfBoundsException} if it is not. diff --git a/java/core/src/main/java/com/google/protobuf/FloatArrayList.java b/java/core/src/main/java/com/google/protobuf/FloatArrayList.java index 4a84b92f5dcb..a9479ad657c5 100644 --- a/java/core/src/main/java/com/google/protobuf/FloatArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/FloatArrayList.java @@ -169,8 +169,7 @@ public void add(int index, Float element) { public void addFloat(float element) { ensureIsMutable(); if (size == array.length) { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); float[] newArray = new float[length]; System.arraycopy(array, 0, newArray, 0, size); @@ -191,8 +190,7 @@ private void addFloat(int index, float element) { // Shift everything over to make room System.arraycopy(array, index, array, index + 1, size - index); } else { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); float[] newArray = new float[length]; // Copy the first part directly @@ -254,6 +252,26 @@ public Float remove(int index) { return value; } + /** Ensures the backing array can fit at least minCapacity elements. */ + void ensureCapacity(int minCapacity) { + if (minCapacity <= array.length) { + return; + } + // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to + // exactly the requested capacity, but must exponentially grow instead. This is similar + // behaviour to ArrayList. + int n = array.length; + while (n < minCapacity) { + n = growSize(n); + } + array = Arrays.copyOf(array, n); + } + + private static int growSize(int previousSize) { + // Resize to 1.5x the size + return ((previousSize * 3) / 2) + 1; + } + /** * Ensures that the provided {@code index} is within the range of {@code [0, size]}. Throws an * {@link IndexOutOfBoundsException} if it is not. diff --git a/java/core/src/main/java/com/google/protobuf/IntArrayList.java b/java/core/src/main/java/com/google/protobuf/IntArrayList.java index e6a1aca1ec7f..f82166948993 100644 --- a/java/core/src/main/java/com/google/protobuf/IntArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/IntArrayList.java @@ -169,8 +169,7 @@ public void add(int index, Integer element) { public void addInt(int element) { ensureIsMutable(); if (size == array.length) { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); int[] newArray = new int[length]; System.arraycopy(array, 0, newArray, 0, size); @@ -191,8 +190,7 @@ private void addInt(int index, int element) { // Shift everything over to make room System.arraycopy(array, index, array, index + 1, size - index); } else { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); int[] newArray = new int[length]; // Copy the first part directly @@ -254,6 +252,26 @@ public Integer remove(int index) { return value; } + /** Ensures the backing array can fit at least minCapacity elements. */ + void ensureCapacity(int minCapacity) { + if (minCapacity <= array.length) { + return; + } + // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to + // exactly the requested capacity, but must exponentially grow instead. This is similar + // behaviour to ArrayList. + int n = array.length; + while (n < minCapacity) { + n = growSize(n); + } + array = Arrays.copyOf(array, n); + } + + private static int growSize(int previousSize) { + // Resize to 1.5x the size + return ((previousSize * 3) / 2) + 1; + } + /** * Ensures that the provided {@code index} is within the range of {@code [0, size]}. Throws an * {@link IndexOutOfBoundsException} if it is not. diff --git a/java/core/src/main/java/com/google/protobuf/LongArrayList.java b/java/core/src/main/java/com/google/protobuf/LongArrayList.java index b85398453e1f..417ee1f7add5 100644 --- a/java/core/src/main/java/com/google/protobuf/LongArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/LongArrayList.java @@ -169,8 +169,7 @@ public void add(int index, Long element) { public void addLong(long element) { ensureIsMutable(); if (size == array.length) { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); long[] newArray = new long[length]; System.arraycopy(array, 0, newArray, 0, size); @@ -191,8 +190,7 @@ private void addLong(int index, long element) { // Shift everything over to make room System.arraycopy(array, index, array, index + 1, size - index); } else { - // Resize to 1.5x the size - int length = ((size * 3) / 2) + 1; + int length = growSize(array.length); long[] newArray = new long[length]; // Copy the first part directly @@ -254,6 +252,26 @@ public Long remove(int index) { return value; } + /** Ensures the backing array can fit at least minCapacity elements. */ + void ensureCapacity(int minCapacity) { + if (minCapacity <= array.length) { + return; + } + // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to + // exactly the requested capacity, but must exponentially grow instead. This is similar + // behaviour to ArrayList. + int n = array.length; + while (n < minCapacity) { + n = growSize(n); + } + array = Arrays.copyOf(array, n); + } + + private static int growSize(int previousSize) { + // Resize to 1.5x the size + return ((previousSize * 3) / 2) + 1; + } + /** * Ensures that the provided {@code index} is within the range of {@code [0, size]}. Throws an * {@link IndexOutOfBoundsException} if it is not. diff --git a/java/core/src/test/java/com/google/protobuf/ArrayDecodersTest.java b/java/core/src/test/java/com/google/protobuf/ArrayDecodersTest.java index b80c9ceaffe4..04cb6f2bce45 100644 --- a/java/core/src/test/java/com/google/protobuf/ArrayDecodersTest.java +++ b/java/core/src/test/java/com/google/protobuf/ArrayDecodersTest.java @@ -159,6 +159,42 @@ public void testDecodePackedFixed32List_negativeSize() { packedSizeBytesNoTag(-1), 0, new IntArrayList(), registers)); } + @Test + public void testDecodePackedFixed32List_2gb_beyondEndOfArray() { + assertThrows( + InvalidProtocolBufferException.class, + () -> + ArrayDecoders.decodePackedFixed32List( + packedSizeBytesNoTag(2_000_000_000), 0, new IntArrayList(), registers)); + } + + @Test + public void testDecodePackedFixed64List_2gb_beyondEndOfArray() { + assertThrows( + InvalidProtocolBufferException.class, + () -> + ArrayDecoders.decodePackedFixed64List( + packedSizeBytesNoTag(2_000_000_000), 0, new LongArrayList(), registers)); + } + + @Test + public void testDecodePackedFloatList_2gb_beyondEndOfArray() { + assertThrows( + InvalidProtocolBufferException.class, + () -> + ArrayDecoders.decodePackedFloatList( + packedSizeBytesNoTag(2_000_000_000), 0, new FloatArrayList(), registers)); + } + + @Test + public void testDecodePackedDoubleList_2gb_beyondEndOfArray() { + assertThrows( + InvalidProtocolBufferException.class, + () -> + ArrayDecoders.decodePackedDoubleList( + packedSizeBytesNoTag(2_000_000_000), 0, new DoubleArrayList(), registers)); + } + @Test public void testDecodePackedFixed64List_negativeSize() { assertThrows(