diff --git a/rskj-core/src/main/java/org/ethereum/util/RLP.java b/rskj-core/src/main/java/org/ethereum/util/RLP.java index 23b4ea69820..5caf38f071f 100644 --- a/rskj-core/src/main/java/org/ethereum/util/RLP.java +++ b/rskj-core/src/main/java/org/ethereum/util/RLP.java @@ -155,24 +155,33 @@ private static byte decodeOneByteItem(byte[] data, int index) { } public static int decodeInt(byte[] data, int index) { - int value = 0; // NOTE: there are two ways zero can be encoded - 0x00 and OFFSET_SHORT_ITEM - if ((data[index] & 0xFF) < OFFSET_SHORT_ITEM) { + if (index < 0 || index >= data.length) { + throw new RLPException("Index out of bounds"); + } + + int firstByte = data[index] & 0xFF; + + if (firstByte < OFFSET_SHORT_ITEM) { return data[index]; - } else if ((data[index] & 0xFF) >= OFFSET_SHORT_ITEM - && (data[index] & 0xFF) < OFFSET_LONG_ITEM) { + } + + if (firstByte < OFFSET_LONG_ITEM) { + int value = 0; byte length = (byte) (data[index] - OFFSET_SHORT_ITEM); + if(length >= (data.length - index)) { + throw new RLPException("RLP wrong encoding"); + } byte pow = (byte) (length - 1); for (int i = 1; i <= length; ++i) { value += (data[index + i] & 0xFF) << (8 * pow); pow--; } - } else { - throw new RuntimeException("wrong decode attempt"); + return value; } - return value; + throw new RLPException("wrong decode attempt"); } public static BigInteger decodeBigInteger(byte[] data, int index) { @@ -191,46 +200,6 @@ public static BigInteger decodeBigInteger(byte[] data, int index) { return BigIntegers.fromUnsignedByteArray(bytes); } - public static byte[] decodeIP4Bytes(byte[] data, int index) { - - int offset = 1; - - final byte[] result = new byte[4]; - for (int i = 0; i < 4; i++) { - result[i] = decodeOneByteItem(data, index + offset); - if ((data[index + offset] & 0xFF) > OFFSET_SHORT_ITEM) { - offset += 2; - } else { - offset += 1; - } - } - - // return IP address - return result; - } - - public static int getFirstListElement(byte[] payload, int pos) { - - if (pos >= payload.length) { - return -1; - } - - if ((payload[pos] & 0xFF) >= OFFSET_LONG_LIST) { - byte lengthOfLength = (byte) (payload[pos] - OFFSET_LONG_LIST); - return pos + lengthOfLength + 1; - } - if ((payload[pos] & 0xFF) >= OFFSET_SHORT_LIST - && (payload[pos] & 0xFF) < OFFSET_LONG_LIST) { - return pos + 1; - } - if ((payload[pos] & 0xFF) >= OFFSET_LONG_ITEM - && (payload[pos] & 0xFF) < OFFSET_SHORT_LIST) { - byte lengthOfLength = (byte) (payload[pos] - OFFSET_LONG_ITEM); - return pos + lengthOfLength + 1; - } - return -1; - } - public static int getNextElementIndex(byte[] payload, int pos) { if (pos >= payload.length) { @@ -426,25 +395,22 @@ private static Pair decodeElement(byte[] msgData, int posit if (b0 <= 192 + TINY_SIZE) { length = b0 - 192 + 1; offset = 1; - } - else { + } else { int nbytes = b0 - 247; - length = 1 + nbytes + bytesToLength(msgData, position + 1, nbytes); offset = 1 + nbytes; + length = safeAdd(offset, bytesToLength(msgData, position + 1, nbytes)); } - if (Long.compareUnsigned(length, Integer.MAX_VALUE) > 0) { - throw new RLPException("The current implementation doesn't support lengths longer than Integer.MAX_VALUE because that is the largest number of elements an array can have"); - } + int endingIndex = safeAdd(length, position); - if (position + length > msgData.length) { + if (endingIndex > msgData.length) { throw new RLPException("The RLP byte array doesn't have enough space to hold an element with the specified length"); } - byte[] bytes = Arrays.copyOfRange(msgData, position, position + length); + byte[] bytes = Arrays.copyOfRange(msgData, position, endingIndex); RLPList list = new RLPList(bytes, offset); - return Pair.of(list, position + length); + return Pair.of(list, endingIndex); } if (b0 == EMPTY_MARK) { @@ -463,8 +429,7 @@ private static Pair decodeElement(byte[] msgData, int posit if (b0 > (EMPTY_MARK + TINY_SIZE)) { offset = b0 - (EMPTY_MARK + TINY_SIZE) + 1; length = bytesToLength(msgData, position + 1, offset - 1); - } - else { + } else { length = b0 & 0x7f; offset = 1; } @@ -473,7 +438,8 @@ private static Pair decodeElement(byte[] msgData, int posit throw new RLPException("The current implementation doesn't support lengths longer than Integer.MAX_VALUE because that is the largest number of elements an array can have"); } - if (position + offset + length < 0 || position + offset + length > msgData.length) { + int endingIndex = position + offset + length; + if ( endingIndex < 0 || endingIndex > msgData.length) { throw new RLPException("The RLP byte array doesn't have enough space to hold an element with the specified length"); } @@ -484,6 +450,14 @@ private static Pair decodeElement(byte[] msgData, int posit return Pair.of(new RLPItem(decoded), position + offset + length); } + private static int safeAdd(int a, int b) { + try{ + return Math.addExact(a, b); + }catch (ArithmeticException ex){ + throw new RLPException("The current implementation doesn't support lengths longer than Integer.MAX_VALUE because that is the largest number of elements an array can have"); + } + } + private static int bytesToLength(byte[] bytes, int position, int size) { if (position + size > bytes.length) { throw new RLPException("The length of the RLP item length can't possibly fit the data byte array"); diff --git a/rskj-core/src/test/java/org/ethereum/util/RLPTest.java b/rskj-core/src/test/java/org/ethereum/util/RLPTest.java index e35526741e3..58bbaa5218f 100644 --- a/rskj-core/src/test/java/org/ethereum/util/RLPTest.java +++ b/rskj-core/src/test/java/org/ethereum/util/RLPTest.java @@ -19,6 +19,8 @@ package org.ethereum.util; +import co.rsk.util.RLPException; +import org.apache.commons.codec.binary.Base64; import org.bouncycastle.util.BigIntegers; import org.bouncycastle.util.encoders.Hex; import org.ethereum.crypto.HashUtil; @@ -34,7 +36,6 @@ import java.io.ObjectOutputStream; import java.io.Serializable; import java.math.BigInteger; -import java.net.InetAddress; import java.net.UnknownHostException; import java.util.*; @@ -47,23 +48,6 @@ @SuppressWarnings("squid:S2699") class RLPTest { - @Test - void test1() throws UnknownHostException { - - String peersPacket = "F8 4E 11 F8 4B C5 36 81 " + - "CC 0A 29 82 76 5F B8 40 D8 D6 0C 25 80 FA 79 5C " + - "FC 03 13 EF DE BA 86 9D 21 94 E7 9E 7C B2 B5 22 " + - "F7 82 FF A0 39 2C BB AB 8D 1B AC 30 12 08 B1 37 " + - "E0 DE 49 98 33 4F 3B CF 73 FA 11 7E F2 13 F8 74 " + - "17 08 9F EA F8 4C 21 B0"; - - byte[] payload = Hex.decode(peersPacket); - - byte[] ip = decodeIP4Bytes(payload, 5); - - assertEquals(InetAddress.getByAddress(ip).toString(), ("/54.204.10.41")); - } - @Test void test2() throws UnknownHostException { @@ -80,60 +64,6 @@ void test2() throws UnknownHostException { assertEquals(30303, oneInt); } - @Test - void test3() throws UnknownHostException { - - String peersPacket = "F8 9A 11 F8 4B C5 36 81 " + - "CC 0A 29 82 76 5F B8 40 D8 D6 0C 25 80 FA 79 5C " + - "FC 03 13 EF DE BA 86 9D 21 94 E7 9E 7C B2 B5 22 " + - "F7 82 FF A0 39 2C BB AB 8D 1B AC 30 12 08 B1 37 " + - "E0 DE 49 98 33 4F 3B CF 73 FA 11 7E F2 13 F8 74 " + - "17 08 9F EA F8 4C 21 B0 F8 4A C4 36 02 0A 29 " + - "82 76 5F B8 40 D8 D6 0C 25 80 FA 79 5C FC 03 13 " + - "EF DE BA 86 9D 21 94 E7 9E 7C B2 B5 22 F7 82 FF " + - "A0 39 2C BB AB 8D 1B AC 30 12 08 B1 37 E0 DE 49 " + - "98 33 4F 3B CF 73 FA 11 7E F2 13 F8 74 17 08 9F " + - "EA F8 4C 21 B0 "; - - byte[] payload = Hex.decode(peersPacket); - - int nextIndex = 5; - byte[] ip = decodeIP4Bytes(payload, nextIndex); - assertEquals("/54.204.10.41", InetAddress.getByAddress(ip).toString()); - - nextIndex = getNextElementIndex(payload, nextIndex); - int port = decodeInt(payload, nextIndex); - assertEquals(30303, port); - - nextIndex = getNextElementIndex(payload, nextIndex); - BigInteger peerId = decodeBigInteger(payload, nextIndex); - - BigInteger expectedPeerId = - new BigInteger("11356629247358725515654715129711890958861491612873043044752814241820167155109073064559464053586837011802513611263556758124445676272172838679152022396871088"); - assertEquals(expectedPeerId, peerId); - - nextIndex = getNextElementIndex(payload, nextIndex); - nextIndex = getFirstListElement(payload, nextIndex); - ip = decodeIP4Bytes(payload, nextIndex); - assertEquals("/54.2.10.41", InetAddress.getByAddress(ip).toString()); - - nextIndex = getNextElementIndex(payload, nextIndex); - port = decodeInt(payload, nextIndex); - assertEquals(30303, port); - - nextIndex = getNextElementIndex(payload, nextIndex); - peerId = decodeBigInteger(payload, nextIndex); - - expectedPeerId = - new BigInteger("11356629247358725515654715129711890958861491612873043044752814241820167155109073064559464053586837011802513611263556758124445676272172838679152022396871088"); - - assertEquals(expectedPeerId, peerId); - - nextIndex = getNextElementIndex(payload, nextIndex); - nextIndex = getFirstListElement(payload, nextIndex); - assertEquals(-1, nextIndex); - } - @Test /** encode byte */ void test4() { @@ -1118,4 +1048,27 @@ void shortStringRightBoundTest() { String res = new String(decode2(rlpEncoded).get(0).getRLPData()); assertEquals(testString, res); } + + @Test + void testDecode2_outOfMem() { + byte[] payload = Base64.decodeBase64("BQACAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/QH4f///9wAAAAAAAAAABSQGYA=="); + assertThrows(RLPException.class, () -> RLP.decode2(payload)); + } + + @Test + void testDecode2() { + byte[] payload = Base64.decodeBase64("ADU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTU1NTX//8DLAH///8s="); + assertThrows(RLPException.class, () -> RLP.decode2(payload)); + } + @Test + void testDecodeInt() { + byte[] payload = Base64.decodeBase64("kQo="); + assertThrows(RLPException.class, () -> RLP.decodeInt(payload, 0)); + } + + @Test + void testIncorrectDecodeInt(){ + byte[] payload = {(byte) 0x84, (byte) 0x00, (byte) 0x00, (byte) 0x84, (byte) 0x00, (byte) 0x0f, (byte) 0xab}; + assertThrows(RLPException.class, () -> RLP.decodeInt(payload, 3)); + } }