Skip to content

Commit

Permalink
Fix UnsafeByteBufUtil#setBytes() cause JVM crash (#10791) (#10795)
Browse files Browse the repository at this point in the history
Motivation:

Passing a null value of byte[] to the `Unsafe.copyMemory(xxx)` would cause the JVM crash 

Modification:

Add null checking before calling `PlatformDependent.copyMemory(src,  xxx)`

Result:

Fixes #10791 .
  • Loading branch information
Ech0Fan authored and normanmaurer committed Nov 16, 2020
1 parent 076aa44 commit 057eb12
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 0 deletions.
7 changes: 7 additions & 0 deletions buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java
Expand Up @@ -532,6 +532,13 @@ static void setBytes(AbstractByteBuf buf, long addr, int index, ByteBuf src, int

static void setBytes(AbstractByteBuf buf, long addr, int index, byte[] src, int srcIndex, int length) {
buf.checkIndex(index, length);
// we need to check not null for src as it may cause the JVM crash
// See https://github.com/netty/netty/issues/10791
checkNotNull(src, "src");
if (isOutOfBounds(srcIndex, length, src.length)) {
throw new IndexOutOfBoundsException("srcIndex: " + srcIndex);
}

if (length != 0) {
PlatformDependent.copyMemory(src, srcIndex, addr, length);
}
Expand Down
117 changes: 117 additions & 0 deletions buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java
Expand Up @@ -90,4 +90,121 @@ public void testSetBytesOnReadOnlyByteBufferWithPooledAlloc() throws Exception {
}
}

@Test
public void testSetBytesWithByteArray() {
final byte[] testData = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
final int length = testData.length;

final UnpooledByteBufAllocator alloc = new UnpooledByteBufAllocator(true);
final UnpooledDirectByteBuf targetBuffer = new UnpooledDirectByteBuf(alloc, length, length);

try {
UnsafeByteBufUtil.setBytes(targetBuffer,
directBufferAddress(targetBuffer.nioBuffer()), 0, testData, 0, length);

final byte[] check = new byte[length];
targetBuffer.getBytes(0, check, 0, length);

assertArrayEquals("The byte array's copy does not equal the original", testData, check);
} finally {
targetBuffer.release();
}
}

@Test
public void testSetBytesWithZeroLength() {
final byte[] testData = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
final int length = testData.length;

final UnpooledByteBufAllocator alloc = new UnpooledByteBufAllocator(true);
final UnpooledDirectByteBuf targetBuffer = new UnpooledDirectByteBuf(alloc, length, length);

try {
final byte[] beforeSet = new byte[length];
targetBuffer.getBytes(0, beforeSet, 0, length);

UnsafeByteBufUtil.setBytes(targetBuffer,
directBufferAddress(targetBuffer.nioBuffer()), 0, testData, 0, 0);

final byte[] check = new byte[length];
targetBuffer.getBytes(0, check, 0, length);

assertArrayEquals(beforeSet, check);
} finally {
targetBuffer.release();
}
}

@Test(expected = NullPointerException.class)
public void testSetBytesWithNullByteArray() {

final UnpooledByteBufAllocator alloc = new UnpooledByteBufAllocator(true);
final UnpooledDirectByteBuf targetBuffer = new UnpooledDirectByteBuf(alloc, 8, 8);

try {
UnsafeByteBufUtil.setBytes(targetBuffer,
directBufferAddress(targetBuffer.nioBuffer()), 0, (byte[]) null, 0, 8);
} finally {
targetBuffer.release();
}
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds() {
// negative index
testSetBytesOutOfBounds0(4, 4, -1, 0, 4);
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds2() {
// negative length
testSetBytesOutOfBounds0(4, 4, 0, 0, -1);
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds3() {
// buffer length oversize
testSetBytesOutOfBounds0(4, 8, 0, 0, 5);
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds4() {
// buffer length oversize
testSetBytesOutOfBounds0(4, 4, 3, 0, 3);
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds5() {
// negative srcIndex
testSetBytesOutOfBounds0(4, 4, 0, -1, 4);
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds6() {
// src length oversize
testSetBytesOutOfBounds0(8, 4, 0, 0, 5);
}

@Test(expected = IndexOutOfBoundsException.class)
public void testSetBytesOutOfBounds7() {
// src length oversize
testSetBytesOutOfBounds0(4, 4, 0, 1, 4);
}

private static void testSetBytesOutOfBounds0(int lengthOfBuffer,
int lengthOfBytes,
int index,
int srcIndex,
int length) {
final UnpooledByteBufAllocator alloc = new UnpooledByteBufAllocator(true);
final UnpooledDirectByteBuf targetBuffer = new UnpooledDirectByteBuf(alloc, lengthOfBuffer, lengthOfBuffer);

try {
UnsafeByteBufUtil.setBytes(targetBuffer,
directBufferAddress(targetBuffer.nioBuffer()), index, new byte[lengthOfBytes], srcIndex, length);
} finally {
targetBuffer.release();
}
}

}

0 comments on commit 057eb12

Please sign in to comment.