Skip to content

Commit

Permalink
Avoid integer overflow if concatenating lists via + operator
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 641298534
Change-Id: I5cc077b537ed51e5bc17889faf898a3517d68ae7
  • Loading branch information
tetromino authored and copybara-github committed Jun 7, 2024
1 parent 242adde commit f7c3184
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,7 @@ private void grow(int mincap) {

// Grow capacity enough to insert given number of elements
private void growAdditional(int additional) throws EvalException {
int mincap = size + additional;
if (mincap < 0 || mincap > MAX_ALLOC) {
throw Starlark.errorf("excessive capacity requested (%d + %d elements)", size, additional);
}
int mincap = addSizesAndFailIfExcessive(size, additional);
grow(mincap);
}

Expand Down
15 changes: 13 additions & 2 deletions src/main/java/net/starlark/java/eval/StarlarkList.java
Original file line number Diff line number Diff line change
Expand Up @@ -204,17 +204,28 @@ public static <T> StarlarkList<T> immutableOf(T... elems) {
/**
* Returns a new {@code StarlarkList} that is the concatenation of two {@code StarlarkList}s. The
* new list will have the given {@link Mutability}.
*
* @throws EvalException if the resulting list would be too large
*/
public static <T> StarlarkList<T> concat(
StarlarkList<? extends T> x, StarlarkList<? extends T> y, Mutability mutability) {
StarlarkList<? extends T> x, StarlarkList<? extends T> y, Mutability mutability)
throws EvalException {
int xsize = x.size();
int ysize = y.size();
Object[] res = new Object[xsize + ysize];
Object[] res = new Object[addSizesAndFailIfExcessive(xsize, ysize)];
System.arraycopy(x.elems(), 0, res, 0, xsize);
System.arraycopy(y.elems(), 0, res, xsize, ysize);
return wrap(mutability, res);
}

protected static int addSizesAndFailIfExcessive(int xsize, int ysize) throws EvalException {
int sum = xsize + ysize;
if (sum < 0 || sum > MAX_ALLOC) {
throw Starlark.errorf("excessive capacity requested (%d + %d elements)", xsize, ysize);
}
return sum;
}

@Nonnull
@Override
public Iterator<E> iterator() {
Expand Down
6 changes: 5 additions & 1 deletion src/test/java/net/starlark/java/eval/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ java_test(
"StarlarkThreadDebuggingTest.java",
"StarlarkThreadTest.java",
],
jvm_flags = ["-Dfile.encoding=UTF8"],
jvm_flags = [
"-Dfile.encoding=UTF8",
# StarlarkListTest.concat_failsCleanlyOnOverflow() needs at least 4GB of max heap
"-Xmx4096m",
],
deps = [
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
Expand Down
14 changes: 14 additions & 0 deletions src/test/java/net/starlark/java/eval/StarlarkListTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,20 @@ public void testTupleToArray() {
}
}

@Test
public void concat_failsCleanlyOnOverflow() throws Exception {
System.gc();
Mutability mu = Mutability.create("test");
// veryBigArray is large enough that veryBigArray.size * 2 > StarlarkList.MAX_ALLOC
Object[] veryBigArray = new Object[(1 << 29) + 1]; // This will OOM if jvm max heap is too low.
StarlarkList<Object> veryBigList = StarlarkList.wrap(mu, veryBigArray);
EvalException e =
assertThrows(EvalException.class, () -> StarlarkList.concat(veryBigList, veryBigList, mu));
assertThat(e)
.hasMessageThat()
.isEqualTo("excessive capacity requested (536870913 + 536870913 elements)");
}

@FormatMethod
private static void fail(@FormatString String format, Object... args) {
throw new AssertionError(String.format(format, args));
Expand Down

0 comments on commit f7c3184

Please sign in to comment.