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:
parent
32e32fe98f
commit
232c669fa4
@ -280,6 +280,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) {
|
||||||
@ -360,6 +366,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 {
|
||||||
|
@ -24,6 +24,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;
|
||||||
@ -1534,4 +1535,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();
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user