AbstractByteBuf readSlice bound check bug
Motivation: AbstractByteBuf#readSlice relied upon the bounds checking of the slice operation in order to detect index out of bounds conditions. However the slice bounds checking operation allows for the slice to go beyond the writer index, and this is out of bounds for a read operation. Modifications: - AbstractByteBuf#readSlice and AbstractByteBuf#readRetainedSlice should ensure the desired amount of bytes are readable before taking a slice Result: No reading of undefined data in AbstractByteBuf#readSlice and AbstractByteBuf#readRetainedSlice.
This commit is contained in:
parent
78522cf3f0
commit
a8bb9dc180
@ -853,6 +853,7 @@ public abstract class AbstractByteBuf extends ByteBuf {
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public ByteBuf readSlice(int length) {
|
public ByteBuf readSlice(int length) {
|
||||||
|
checkReadableBytes(length);
|
||||||
ByteBuf slice = slice(readerIndex, length);
|
ByteBuf slice = slice(readerIndex, length);
|
||||||
readerIndex += length;
|
readerIndex += length;
|
||||||
return slice;
|
return slice;
|
||||||
@ -860,6 +861,7 @@ public abstract class AbstractByteBuf extends ByteBuf {
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public ByteBuf readRetainedSlice(int length) {
|
public ByteBuf readRetainedSlice(int length) {
|
||||||
|
checkReadableBytes(length);
|
||||||
ByteBuf slice = retainedSlice(readerIndex, length);
|
ByteBuf slice = retainedSlice(readerIndex, length);
|
||||||
readerIndex += length;
|
readerIndex += length;
|
||||||
return slice;
|
return slice;
|
||||||
|
@ -3286,6 +3286,31 @@ public abstract class AbstractByteBufTest {
|
|||||||
assertEquals(0, buf.refCnt());
|
assertEquals(0, buf.refCnt());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test(expected = IndexOutOfBoundsException.class)
|
||||||
|
public void testReadSliceOutOfBounds() {
|
||||||
|
testReadSliceOutOfBounds(false);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IndexOutOfBoundsException.class)
|
||||||
|
public void testReadRetainedSliceOutOfBounds() {
|
||||||
|
testReadSliceOutOfBounds(true);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void testReadSliceOutOfBounds(boolean retainedSlice) {
|
||||||
|
ByteBuf buf = newBuffer(100);
|
||||||
|
try {
|
||||||
|
buf.writeZero(50);
|
||||||
|
if (retainedSlice) {
|
||||||
|
buf.readRetainedSlice(51);
|
||||||
|
} else {
|
||||||
|
buf.readSlice(51);
|
||||||
|
}
|
||||||
|
fail();
|
||||||
|
} finally {
|
||||||
|
buf.release();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWriteUsAsciiCharSequenceExpand() {
|
public void testWriteUsAsciiCharSequenceExpand() {
|
||||||
testWriteCharSequenceExpand(CharsetUtil.US_ASCII);
|
testWriteCharSequenceExpand(CharsetUtil.US_ASCII);
|
||||||
|
@ -25,6 +25,7 @@ import java.util.Queue;
|
|||||||
|
|
||||||
import static org.junit.Assert.assertSame;
|
import static org.junit.Assert.assertSame;
|
||||||
import static org.junit.Assert.assertTrue;
|
import static org.junit.Assert.assertTrue;
|
||||||
|
import static org.junit.Assume.assumeTrue;
|
||||||
|
|
||||||
public class SimpleLeakAwareByteBufTest extends BigEndianHeapByteBufTest {
|
public class SimpleLeakAwareByteBufTest extends BigEndianHeapByteBufTest {
|
||||||
private final Class<? extends ByteBuf> clazz = leakClass();
|
private final Class<? extends ByteBuf> clazz = leakClass();
|
||||||
@ -84,7 +85,12 @@ public class SimpleLeakAwareByteBufTest extends BigEndianHeapByteBufTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWrapReadSlice() {
|
public void testWrapReadSlice() {
|
||||||
assertWrapped(newBuffer(8).readSlice(1));
|
ByteBuf buffer = newBuffer(8);
|
||||||
|
if (buffer.isReadable()) {
|
||||||
|
assertWrapped(buffer.readSlice(1));
|
||||||
|
} else {
|
||||||
|
assertTrue(buffer.release());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@ -97,14 +103,18 @@ public class SimpleLeakAwareByteBufTest extends BigEndianHeapByteBufTest {
|
|||||||
@Test
|
@Test
|
||||||
public void testWrapRetainedSlice2() {
|
public void testWrapRetainedSlice2() {
|
||||||
ByteBuf buffer = newBuffer(8);
|
ByteBuf buffer = newBuffer(8);
|
||||||
assertWrapped(buffer.retainedSlice(0, 1));
|
if (buffer.isReadable()) {
|
||||||
|
assertWrapped(buffer.retainedSlice(0, 1));
|
||||||
|
}
|
||||||
assertTrue(buffer.release());
|
assertTrue(buffer.release());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWrapReadRetainedSlice() {
|
public void testWrapReadRetainedSlice() {
|
||||||
ByteBuf buffer = newBuffer(8);
|
ByteBuf buffer = newBuffer(8);
|
||||||
assertWrapped(buffer.readRetainedSlice(1));
|
if (buffer.isReadable()) {
|
||||||
|
assertWrapped(buffer.readRetainedSlice(1));
|
||||||
|
}
|
||||||
assertTrue(buffer.release());
|
assertTrue(buffer.release());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -81,7 +81,12 @@ public class SimpleLeakAwareCompositeByteBufTest extends WrappedCompositeByteBuf
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWrapReadSlice() {
|
public void testWrapReadSlice() {
|
||||||
assertWrapped(newBuffer(8).readSlice(1));
|
ByteBuf buffer = newBuffer(8);
|
||||||
|
if (buffer.isReadable()) {
|
||||||
|
assertWrapped(buffer.readSlice(1));
|
||||||
|
} else {
|
||||||
|
assertTrue(buffer.release());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@ -94,14 +99,18 @@ public class SimpleLeakAwareCompositeByteBufTest extends WrappedCompositeByteBuf
|
|||||||
@Test
|
@Test
|
||||||
public void testWrapRetainedSlice2() {
|
public void testWrapRetainedSlice2() {
|
||||||
ByteBuf buffer = newBuffer(8);
|
ByteBuf buffer = newBuffer(8);
|
||||||
assertWrapped(buffer.retainedSlice(0, 1));
|
if (buffer.isReadable()) {
|
||||||
|
assertWrapped(buffer.retainedSlice(0, 1));
|
||||||
|
}
|
||||||
assertTrue(buffer.release());
|
assertTrue(buffer.release());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWrapReadRetainedSlice() {
|
public void testWrapReadRetainedSlice() {
|
||||||
ByteBuf buffer = newBuffer(8);
|
ByteBuf buffer = newBuffer(8);
|
||||||
assertWrapped(buffer.readRetainedSlice(1));
|
if (buffer.isReadable()) {
|
||||||
|
assertWrapped(buffer.readRetainedSlice(1));
|
||||||
|
}
|
||||||
assertTrue(buffer.release());
|
assertTrue(buffer.release());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -641,7 +641,9 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
|
|||||||
prevPadding = padding;
|
prevPadding = padding;
|
||||||
flags.paddingPresent(padding > 0);
|
flags.paddingPresent(padding > 0);
|
||||||
flags.endOfStream(endOfStream);
|
flags.endOfStream(endOfStream);
|
||||||
frameHeader = buffer.readSlice(DATA_FRAME_HEADER_LENGTH).writerIndex(0);
|
frameHeader = buffer.slice(buffer.readerIndex(), DATA_FRAME_HEADER_LENGTH).writerIndex(0);
|
||||||
|
buffer.setIndex(buffer.readerIndex() + DATA_FRAME_HEADER_LENGTH,
|
||||||
|
buffer.writerIndex() + DATA_FRAME_HEADER_LENGTH);
|
||||||
|
|
||||||
int payloadLength = data + padding;
|
int payloadLength = data + padding;
|
||||||
writeFrameHeaderInternal(frameHeader, payloadLength, DATA, flags, streamId);
|
writeFrameHeaderInternal(frameHeader, payloadLength, DATA, flags, streamId);
|
||||||
|
Loading…
Reference in New Issue
Block a user