Guard against overflow when calling CompositeByteBuf.addComponent(...) (#10197)

Motivation:

We need to ensure we not overflow when adding buffers to a CompositeByteBuf

Modifications:

- Correctly validate overflow before adding to the internal storage
- Add testcase

Result:

Fixes https://github.com/netty/netty/issues/10194
This commit is contained in:
Norman Maurer 2020-04-21 12:04:25 +02:00
parent 586eca2809
commit 469c1ca34c
2 changed files with 70 additions and 0 deletions

View File

@ -279,6 +279,12 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
Component c = newComponent(ensureAccessible(buffer), 0); Component c = newComponent(ensureAccessible(buffer), 0);
int readableBytes = c.length(); int readableBytes = c.length();
// Check if we would overflow.
// See https://github.com/netty/netty/issues/10194
if (capacity() + readableBytes < 0) {
throw new IllegalArgumentException("Can't increase by " + readableBytes);
}
addComp(cIndex, c); addComp(cIndex, c);
wasAdded = true; wasAdded = true;
if (readableBytes > 0 && cIndex < componentCount - 1) { if (readableBytes > 0 && cIndex < componentCount - 1) {
@ -359,6 +365,18 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
private CompositeByteBuf addComponents0(boolean increaseWriterIndex, private CompositeByteBuf addComponents0(boolean increaseWriterIndex,
final int cIndex, ByteBuf[] buffers, int arrOffset) { final int cIndex, ByteBuf[] buffers, int arrOffset) {
final int len = buffers.length, count = len - arrOffset; final int len = buffers.length, count = len - arrOffset;
int readableBytes = 0;
int capacity = capacity();
for (int i = 0; i < buffers.length; i++) {
readableBytes += buffers[i].readableBytes();
// Check if we would overflow.
// See https://github.com/netty/netty/issues/10194
if (capacity + readableBytes < 0) {
throw new IllegalArgumentException("Can't increase by " + readableBytes);
}
}
// only set ci after we've shifted so that finally block logic is always correct // only set ci after we've shifted so that finally block logic is always correct
int ci = Integer.MAX_VALUE; int ci = Integer.MAX_VALUE;
try { try {

View File

@ -22,6 +22,7 @@ import org.junit.Test;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.nio.ByteOrder; import java.nio.ByteOrder;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.ConcurrentModificationException; import java.util.ConcurrentModificationException;
import java.util.Iterator; import java.util.Iterator;
@ -1524,4 +1525,55 @@ public abstract class AbstractCompositeByteBufTest extends AbstractByteBufTest {
} }
assertTrue(cbuf.release()); assertTrue(cbuf.release());
} }
@Test(expected = IllegalArgumentException.class)
public void testOverflowWhileAddingComponent() {
int capacity = 1024 * 1024; // 1MB
ByteBuf buffer = Unpooled.buffer(capacity).writeZero(capacity);
CompositeByteBuf compositeByteBuf = compositeBuffer(Integer.MAX_VALUE);
try {
for (int i = 0; i >= 0; i += buffer.readableBytes()) {
ByteBuf duplicate = buffer.duplicate();
compositeByteBuf.addComponent(duplicate);
duplicate.retain();
}
} finally {
compositeByteBuf.release();
}
}
@Test(expected = IllegalArgumentException.class)
public void testOverflowWhileAddingComponentsViaVarargs() {
int capacity = 1024 * 1024; // 1MB
ByteBuf buffer = Unpooled.buffer(capacity).writeZero(capacity);
CompositeByteBuf compositeByteBuf = compositeBuffer(Integer.MAX_VALUE);
try {
for (int i = 0; i >= 0; i += buffer.readableBytes()) {
ByteBuf duplicate = buffer.duplicate();
compositeByteBuf.addComponents(duplicate);
duplicate.retain();
}
} finally {
compositeByteBuf.release();
}
}
@Test(expected = IllegalArgumentException.class)
public void testOverflowWhileAddingComponentsViaIterable() {
int capacity = 1024 * 1024; // 1MB
ByteBuf buffer = Unpooled.buffer(capacity).writeZero(capacity);
CompositeByteBuf compositeByteBuf = compositeBuffer(Integer.MAX_VALUE);
try {
for (int i = 0; i >= 0; i += buffer.readableBytes()) {
ByteBuf duplicate = buffer.duplicate();
compositeByteBuf.addComponents(Collections.singletonList(duplicate));
duplicate.retain();
}
} finally {
compositeByteBuf.release();
}
}
} }