From 057eb121f4b1b31db590f518f7e7f4c8ac7480ab Mon Sep 17 00:00:00 2001 From: Ech0Fan <65644290+Ech0Fan@users.noreply.github.com> Date: Mon, 16 Nov 2020 16:01:01 +0800 Subject: [PATCH] Fix UnsafeByteBufUtil#setBytes() cause JVM crash (#10791) (#10795) 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 . --- .../io/netty/buffer/UnsafeByteBufUtil.java | 7 ++ .../netty/buffer/UnsafeByteBufUtilTest.java | 117 ++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java index 6cfa835b646..0623d7b1d44 100644 --- a/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java @@ -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); } diff --git a/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java b/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java index 723568ea820..e5cd26d578b 100644 --- a/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java +++ b/buffer/src/test/java/io/netty/buffer/UnsafeByteBufUtilTest.java @@ -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(); + } + } + }