Skip to content

Commit

Permalink
Allocate correct-sized array when parsing packed fixed-width primitives
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mhansen @copybara-github
mhansen authored and copybara-github committed Sep 12, 2024
1 parent ae51a89 commit 4e8469c
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 24 deletions.
28 changes: 24 additions & 4 deletions java/core/src/main/java/com/google/protobuf/ArrayDecoders.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
26 changes: 22 additions & 4 deletions java/core/src/main/java/com/google/protobuf/BooleanArrayList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 22 additions & 4 deletions java/core/src/main/java/com/google/protobuf/DoubleArrayList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 22 additions & 4 deletions java/core/src/main/java/com/google/protobuf/FloatArrayList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 22 additions & 4 deletions java/core/src/main/java/com/google/protobuf/IntArrayList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 22 additions & 4 deletions java/core/src/main/java/com/google/protobuf/LongArrayList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
36 changes: 36 additions & 0 deletions java/core/src/test/java/com/google/protobuf/ArrayDecodersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 4e8469c

Please sign in to comment.