Make byteArrayToLong negative safe

rsksmart · Nov 22, 2023 · 026f190 · 026f190
1 parent bd8d819
commit 026f190
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 22 deletions.
diff --git a/rskj-core/src/main/java/org/ethereum/util/ByteUtil.java b/rskj-core/src/main/java/org/ethereum/util/ByteUtil.java
@@ -337,29 +337,27 @@ public static int byteArrayToInt(byte[] b) {
     /**
      * Cast from byte[] to long
      * Limited to Long.MAX_VALUE: 2^63-1 (8 bytes)
-     * Will throw IlegalArgumentException if you  pass
+     * Will throw RuntimeException if you  pass
      * a byte array with more than 8 bytes.
      *
      * @param b array contains the values
      * @return unsigned positive long value.
      */
     public static long byteArrayToLong(byte[] b) {
         if (b == null || b.length == 0) {
-             return 0;
-        }
-        // avoids overflows in the result
-        if (b.length > 8) {
-            throw new IllegalArgumentException("byte array can't have more than 8 bytes if it is to be cast to long");
+            return 0;
         }
-        long result = 0;
-        for (int i = 0; i < b.length; i++) {
-            result <<= 8;
-            result |= (b[i] & 0xFF);
+
+        BigInteger bigInteger = new BigInteger(1, b);
+
+        if (bigInteger.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) > 0) {
+            throw new RuntimeException("Cannot represent Integer " + bigInteger.toString(16));
         }
-        return result;
-    }
 
 
+        return bigInteger.longValue();
+    }
+
     /**
      * Turn nibbles to a pretty looking output string
      *

diff --git a/rskj-core/src/main/java/org/ethereum/vm/GasCost.java b/rskj-core/src/main/java/org/ethereum/vm/GasCost.java
@@ -135,14 +135,11 @@ private GasCost() { }
      *                             is negative.
      */
     public static long toGas(byte[] bytes) throws InvalidGasException {
-        if (bytes.length > 8) {
+        if (bytes.length > 8 || (bytes.length == 8 && (bytes[0] & 0xff) >= 0x80)) {
             return Long.MAX_VALUE;
         }
-        long result = ByteUtil.byteArrayToLong(bytes);
-        if (result < 0) {
-            throw new InvalidGasException(bytes);
-        }
-        return result;
+
+        return ByteUtil.byteArrayToLong(bytes);
     }
 
     /**

diff --git a/rskj-core/src/test/java/org/ethereum/util/ByteUtilTest.java b/rskj-core/src/test/java/org/ethereum/util/ByteUtilTest.java
@@ -97,13 +97,29 @@ void testByteArrayToLong() {
 
     @Test
     void testByteArrayToLongThrowsWhenOverflow() {
-        Assertions.assertThrows(IllegalArgumentException.class, () -> ByteUtil.byteArrayToLong(new byte[]{
+        Assertions.assertThrows(RuntimeException.class, () -> ByteUtil.byteArrayToLong(new byte[]{
                 (byte)255, (byte)255, (byte)255, (byte)255,
                 (byte)255, (byte)255, (byte)255, (byte)255,
                 (byte)123, }
         ));
     }
 
+    @Test
+    void testByteArrayToLongNegative() {
+        Assertions.assertThrows(RuntimeException.class, () -> ByteUtil.byteArrayToLong(new byte[]{
+                (byte)-1, (byte)255, (byte)255, (byte)255,
+                (byte)255, (byte)255, (byte)255, (byte)255
+        }));
+    }
+
+    @Test
+    void testByteArrayToLongNegative2() {
+        Assertions.assertThrows(RuntimeException.class, () -> ByteUtil.byteArrayToLong(new byte[]{
+                (byte) 0xa4, (byte) 0xbe, (byte) 0xfa, (byte) 0xd1,
+                (byte) 0x41, (byte) 0xd5, (byte) 0x1c, (byte) 0x4f
+        }));
+    }
+
     @Test
     void testByteArrayToInt() {
         Assertions.assertEquals(0, ByteUtil.byteArrayToInt(null));

diff --git a/rskj-core/src/test/java/org/ethereum/vm/GasCostTest.java b/rskj-core/src/test/java/org/ethereum/vm/GasCostTest.java
@@ -53,14 +53,47 @@ void toGasOverflowsSlightly() {
         Assertions.assertEquals(Long.MAX_VALUE, GasCost.toGas(bytes));
     }
 
+    @Test
+    void toGasInRange() throws GasCost.InvalidGasException {
+        Assertions.assertEquals(0L, GasCost.toGas(new byte[]{
+                (byte)0
+        }));
+
+
+        Assertions.assertEquals(0x000000000a141e28L, GasCost.toGas(new byte[]{
+                (byte)10, (byte)20, (byte)30, (byte)40,
+        }));
+
+        Assertions.assertEquals(0x7ffffffffffffffeL, GasCost.toGas(new byte[]{
+                (byte)0x7f, (byte)255, (byte)255, (byte)255,
+                (byte)255, (byte)255, (byte)255, (byte)254,
+        }));
+
+        Assertions.assertEquals(Long.MAX_VALUE, GasCost.toGas(new byte[]{
+                (byte)0x7f, (byte)255, (byte)255, (byte)255,
+                (byte)255, (byte)255, (byte)255, (byte)255,
+        }));
+    }
+
+    @Test
+    void toGasIsAlwaysPositive() throws GasCost.InvalidGasException {
+        Assertions.assertEquals(255, GasCost.toGas(new byte[]{
+                (byte) (-1 & 0xff),
+        }));
+
+        Assertions.assertEquals(Long.MAX_VALUE, GasCost.toGas(new byte[]{
+                (byte) (-1 & 0xff), (byte)255, (byte)255, (byte)255,
+                (byte)255, (byte)255, (byte)255, (byte)255,
+        }));
+    }
 
     @Test
-    void toGasGivesNegativeValue() throws GasCost.InvalidGasException {
+    void toGasGivesMaxIntegerOnOverflow() throws GasCost.InvalidGasException {
         byte[] negativeBytes = new byte[]{
                 (byte)255, (byte)255, (byte)255, (byte)255,
                 (byte)255, (byte)255, (byte)255, (byte)255,
         };
-        Assertions.assertThrows(GasCost.InvalidGasException.class, () -> GasCost.toGas(negativeBytes));
+        Assertions.assertEquals(Long.MAX_VALUE, GasCost.toGas(negativeBytes));
     }
 
     @Test

diff --git a/rskj-core/src/test/resources/json/StateTests/stSystemOperationsTest.json b/rskj-core/src/test/resources/json/StateTests/stSystemOperationsTest.json
@@ -7874,7 +7874,7 @@
             "currentDifficulty" : "0x7fb7d889155ce8c6",
             "currentGasLimit" : "0x58272e28",
             "currentNumber" : "0xa7d8c0",
-            "currentTimestamp" : "0xa4befad141d51c4f",
+            "currentTimestamp" : "0x00",
             "previousHash" : "d30f77155de00f207ad60109897e790f73e9f3431be25717bf3651d91949f041"
         },
         "logs" : [