diff --git a/abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java b/abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java index 94e0181b..8b345fbc 100644 --- a/abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java +++ b/abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java @@ -146,7 +146,13 @@ public static int getDataOffset( String input, int offset, TypeReference typeReference) throws ClassNotFoundException { if (isDynamic(typeReference)) { - return TypeDecoder.decodeUintAsInt(input, offset) << 1; + int dataOffset = TypeDecoder.decodeUintAsInt(input, offset); + try { + return Math.multiplyExact(dataOffset, 2); + } catch (ArithmeticException e) { + throw new IllegalArgumentException( + "Invalid ABI data offset: " + dataOffset, e); + } } else { return offset; } diff --git a/abi/src/main/java/org/tron/trident/abi/TypeDecoder.java b/abi/src/main/java/org/tron/trident/abi/TypeDecoder.java index 43c5a147..54abe7a2 100644 --- a/abi/src/main/java/org/tron/trident/abi/TypeDecoder.java +++ b/abi/src/main/java/org/tron/trident/abi/TypeDecoder.java @@ -314,12 +314,31 @@ static int getSingleElementLength(String input, int offset, Cla } } + /** + * Rejects reads that would run past the end of the input: {@code windowLength} + * hex chars must exist at {@code offset}. + */ + private static void checkWindowBounds(int inputLength, int offset, int windowLength) { + if (offset < 0 || windowLength < 0 || (long) offset + windowLength > inputLength) { + throw new IllegalArgumentException( + "Invalid ABI input: offset " + offset + " with length " + windowLength + + " out of bounds for length " + inputLength); + } + } + static int decodeUintAsInt(String rawInput, int offset) { + checkWindowBounds(rawInput.length(), offset, MAX_BYTE_LENGTH_FOR_HEX_STRING); String input = rawInput.substring(offset, offset + MAX_BYTE_LENGTH_FOR_HEX_STRING); - return decode(input, 0, Uint.class).getValue().intValue(); + BigInteger value = decode(input, 0, Uint.class).getValue(); + if (value.bitLength() > 31) { + throw new IllegalArgumentException( + "Invalid ABI uint " + value + " exceeds Integer.MAX_VALUE"); + } + return value.intValue(); } public static Bool decodeBool(String rawInput, int offset) { + checkWindowBounds(rawInput.length(), offset, MAX_BYTE_LENGTH_FOR_HEX_STRING); String input = rawInput.substring(offset, offset + MAX_BYTE_LENGTH_FOR_HEX_STRING); BigInteger numericValue = Numeric.toBigInt(input); boolean value = numericValue.equals(BigInteger.ONE); @@ -353,10 +372,18 @@ public static T decodeBytes(String input, int offset, Class public static DynamicBytes decodeDynamicBytes(String input, int offset) { int encodedLength = decodeUintAsInt(input, offset); - int hexStringEncodedLength = encodedLength << 1; - int valueOffset = offset + MAX_BYTE_LENGTH_FOR_HEX_STRING; + // Reject lengths that cannot physically fit in the remaining input. + // Also stops `encodedLength << 1` from flipping the sign bit when + // encodedLength is in the upper half of the int domain. + int remainingHex = input.length() - valueOffset; + if (encodedLength < 0 || encodedLength > remainingHex / 2) { + throw new IllegalArgumentException( + "Invalid ABI dynamic bytes length: " + encodedLength); + } + + int hexStringEncodedLength = encodedLength << 1; String data = input.substring(valueOffset, valueOffset + hexStringEncodedLength); byte[] bytes = Numeric.hexStringToByteArray(data); @@ -446,6 +473,7 @@ private static T decodeStaticStructElementFromInnerTypes( currOffset += (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) * MAX_BYTE_LENGTH_FOR_HEX_STRING; } else { + checkWindowBounds(input.length(), currOffset, MAX_BYTE_LENGTH_FOR_HEX_STRING); value = decode(input.substring(currOffset, currOffset + 64), 0, declaredField); currOffset += MAX_BYTE_LENGTH_FOR_HEX_STRING; } @@ -485,6 +513,7 @@ private static T decodeStaticStructElement( input, currOffset, classType, constructor, i, declaredField); currOffset += value.bytes32PaddedLength() * 2; } else { + checkWindowBounds(input.length(), currOffset, MAX_BYTE_LENGTH_FOR_HEX_STRING); value = decode(input.substring(currOffset, currOffset + 64), 0, declaredField); currOffset += 64; } @@ -627,6 +656,7 @@ ParameterOffsetTracker getDynamicOffsetsAndNonDynamicParameters( final T value; final int beginIndex = offset + tracker.staticOffset; if (isDynamic(innerType)) { + checkWindowBounds(input.length(), beginIndex, MAX_BYTE_LENGTH_FOR_HEX_STRING); final int parameterOffset = decodeDynamicStructDynamicParameterOffset( input.substring(beginIndex, beginIndex + 64)) @@ -635,6 +665,7 @@ ParameterOffsetTracker getDynamicOffsetsAndNonDynamicParameters( tracker.staticOffset += 64; tracker.dynamicParametersToProcess += 1; } else { + checkWindowBounds(input.length(), beginIndex, MAX_BYTE_LENGTH_FOR_HEX_STRING); if (StaticStruct.class.isAssignableFrom(declaredField)) { value = decodeStaticStruct(input, beginIndex, innerType); tracker.staticOffset += (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) @@ -726,6 +757,7 @@ private static T decodeDynamicStructElements( final T value; final int beginIndex = offset + staticOffset; if (isDynamicStructField(constructor, i)) { + checkWindowBounds(input.length(), beginIndex, MAX_BYTE_LENGTH_FOR_HEX_STRING); final int parameterOffset = decodeDynamicStructDynamicParameterOffset( input.substring(beginIndex, beginIndex + 64)) @@ -733,6 +765,9 @@ private static T decodeDynamicStructElements( parameterOffsets.add(parameterOffset); staticOffset += 64; } else { + // Static head fields occupy 64 hex chars each; downstream decodeNumeric + // assumes that, so reject short input here rather than letting arraycopy throw. + checkWindowBounds(input.length(), beginIndex, MAX_BYTE_LENGTH_FOR_HEX_STRING); if (StaticStruct.class.isAssignableFrom(declaredField)) { value = decodeStaticStruct( @@ -839,6 +874,7 @@ private static T decodeDynamicParameterFromStruct( final Class declaredField, final Class parameter) throws ClassNotFoundException { + checkWindowBounds(input.length(), parameterOffset, parameterLength); final String dynamicElementData = input.substring(parameterOffset, parameterOffset + parameterLength); @@ -879,6 +915,7 @@ private static T decodeDynamicParameterFromStructWithTypeRefere final int parameterLength, final TypeReference parameterTypeReference) throws ClassNotFoundException { + checkWindowBounds(input.length(), parameterOffset, parameterLength); final String dynamicElementData = input.substring(parameterOffset, parameterOffset + parameterLength); final Class declaredField = parameterTypeReference.getClassType(); @@ -898,7 +935,13 @@ private static T decodeDynamicParameterFromStructWithTypeRefere } private static int decodeDynamicStructDynamicParameterOffset(final String input) { - return (decodeUintAsInt(input, 0) * 2); + int parameterOffset = decodeUintAsInt(input, 0); + try { + return Math.multiplyExact(parameterOffset, 2); + } catch (ArithmeticException e) { + throw new IllegalArgumentException( + "Invalid ABI dynamic struct parameter offset: " + parameterOffset, e); + } } /** @@ -1004,6 +1047,21 @@ private static T instantiateStaticArray(List elements, int l } } + /** + * Advances an array-element offset by an input-derived element length, computing in long to + * absorb any multiplication that would otherwise overflow int, and rejecting offsets past the + * end of input. + */ + private static int advanceArrayElementOffset( + final String input, final int currOffset, final long elementLengthHex, final int index) { + long nextOffset = (long) currOffset + elementLengthHex; + if (nextOffset < 0 || nextOffset > input.length()) { + throw new IllegalArgumentException( + "Invalid ABI array element offset at index " + index + ": " + nextOffset); + } + return (int) nextOffset; + } + private static T decodeArrayElements( String input, int offset, @@ -1012,6 +1070,15 @@ private static T decodeArrayElements( BiFunction, String, T> consumer) { try { Class cls = Utils.getParameterizedTypeFromArray(typeReference); + int remainingHex = input.length() - offset; + if (offset < 0 + || length < 0 + || remainingHex < 0 + || length > remainingHex / MAX_BYTE_LENGTH_FOR_HEX_STRING) { + throw new IllegalArgumentException( + "Invalid ABI array: length " + length + " at offset " + offset + + " out of bounds for input length " + input.length()); + } List elements = new ArrayList<>(length); if (StructType.class.isAssignableFrom(cls)) { int currOffset = offset; @@ -1029,9 +1096,12 @@ private static T decodeArrayElements( (TypeReference) new TypeReference( typeReference.isIndexed(), typeReference.getSubTypeReference().getInnerTypes()) {}); - currOffset += - getSingleElementLength(input, currOffset, cls) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) getSingleElementLength(input, currOffset, cls) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } else { value = TypeDecoder.decodeDynamicStruct( @@ -1040,9 +1110,12 @@ private static T decodeArrayElements( + getDataOffset( input, currOffset, typeReference), TypeReference.create(cls)); - currOffset += - getSingleElementLength(input, currOffset, cls) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) getSingleElementLength(input, currOffset, cls) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } } else { if (Optional.ofNullable(typeReference) @@ -1053,16 +1126,22 @@ private static T decodeArrayElements( input, currOffset, (TypeReference) typeReference.getSubTypeReference()); - currOffset += - (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } else { value = TypeDecoder.decodeStaticStruct( input, currOffset, TypeReference.create(cls)); - currOffset += - (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } } elements.add(value); @@ -1085,9 +1164,12 @@ private static T decodeArrayElements( + getDataOffset( input, currOffset, typeReference), dynamicTypeRef); - currOffset += - getSingleElementLength(input, currOffset, cls) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) getSingleElementLength(input, currOffset, cls) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } else { // Prefer the size carried by the element's StaticArrayTypeReference: for // sizes with no generated class (e.g. uint256[33]) cls is the bare @@ -1156,9 +1238,12 @@ public java.lang.reflect.Type getOwnerType() { (T) TypeDecoder.decodeStaticArray( input, currOffset, staticReference, staticLength); - currOffset += - (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) (value.bytes32PaddedLength() / Type.MAX_BYTE_LENGTH) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } } elements.add(value); @@ -1174,9 +1259,12 @@ public java.lang.reflect.Type getOwnerType() { currOffset += MAX_BYTE_LENGTH_FOR_HEX_STRING; } else { value = decode(input, currOffset, cls); - currOffset += - getSingleElementLength(input, currOffset, cls) - * MAX_BYTE_LENGTH_FOR_HEX_STRING; + currOffset = advanceArrayElementOffset( + input, + currOffset, + (long) getSingleElementLength(input, currOffset, cls) + * MAX_BYTE_LENGTH_FOR_HEX_STRING, + i); } elements.add(value); } diff --git a/abi/src/test/java/org/tron/trident/abi/TypeDecoderDoSTest.java b/abi/src/test/java/org/tron/trident/abi/TypeDecoderDoSTest.java new file mode 100644 index 00000000..5f21662a --- /dev/null +++ b/abi/src/test/java/org/tron/trident/abi/TypeDecoderDoSTest.java @@ -0,0 +1,407 @@ +/* + * Copyright 2019 Web3 Labs Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package org.tron.trident.abi; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.math.BigInteger; +import java.util.Arrays; + +import org.junit.jupiter.api.Test; +import org.tron.trident.abi.datatypes.DynamicArray; +import org.tron.trident.abi.datatypes.DynamicBytes; +import org.tron.trident.abi.datatypes.DynamicStruct; +import org.tron.trident.abi.datatypes.StaticStruct; +import org.tron.trident.abi.datatypes.Utf8String; +import org.tron.trident.abi.datatypes.generated.Int256; +import org.tron.trident.abi.datatypes.generated.Uint256; + +/** + * Regression tests for decoder hardening against malicious ABI inputs. + */ +public class TypeDecoderDoSTest { + + /** A 32-byte slot whose value is 2^255 — far above Integer.MAX_VALUE. */ + private static final String HUGE_UINT_SLOT = + "8000000000000000000000000000000000000000000000000000000000000000"; + + @Test + public void decodeUintAsInt_rejectsValueExceedingIntMax() { + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeUintAsInt(HUGE_UINT_SLOT, 0)); + } + + @Test + public void decodeDynamicArray_rejectsHugeLength() { + // Length prefix = 2^255. Without the bound, intValue() truncates to + // a negative int and new ArrayList<>(negative) throws — or worse, a + // value just above Int.MAX would OOM the JVM on pre-allocation. + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicArray( + HUGE_UINT_SLOT, 0, new TypeReference>() {})); + } + + @Test + public void decodeDynamicArray_rejectsLengthExceedingRemainingInput() { + // Length = 1000 fits in int (passes the bitLength check) but cannot + // possibly fit in this 1-slot input. The decodeArrayElements cap + // rejects before new ArrayList<>(1000) allocates. + String malicious = + "00000000000000000000000000000000000000000000000000000000000003e8"; // length=1000 + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicArray( + malicious, 0, new TypeReference>() {})); + } + + @Test + public void decodeDynamicBytes_rejectsHugePayloadLength() { + // Length prefix = 2^255 — without the bound, substring tries to read + // 2^255 hex chars and throws SIOOBE. + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicBytes(HUGE_UINT_SLOT, 0)); + } + + @Test + public void decodeUtf8String_rejectsHugePayloadLength() { + // Inherits the same protection via decodeDynamicBytes. + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeUtf8String(HUGE_UINT_SLOT, 0)); + } + + @Test + public void decodeDynamicArray_acceptsLegitimateArray() { + String legit = + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000002"; + DynamicArray result = TypeDecoder.decodeDynamicArray( + legit, 0, new TypeReference>() {}); + assertEquals(2, result.getValue().size()); + } + + @Test + public void decodeDynamicBytes_acceptsLegitimateBytes() { + String legit = + "0000000000000000000000000000000000000000000000000000000000000004" + + "deadbeef00000000000000000000000000000000000000000000000000000000"; + DynamicBytes result = TypeDecoder.decodeDynamicBytes(legit, 0); + assertEquals(4, result.getValue().length); + assertEquals((byte) 0xde, result.getValue()[0]); + assertEquals((byte) 0xef, result.getValue()[3]); + } + + @Test + public void decodeUtf8String_acceptsLegitimateString() { + String legit = + "0000000000000000000000000000000000000000000000000000000000000005" + + "68656c6c6f000000000000000000000000000000000000000000000000000000"; + Utf8String result = TypeDecoder.decodeUtf8String(legit, 0); + assertEquals("hello", result.getValue()); + } + + @Test + public void decodeDynamicBytes_rejectsLengthExceedingRemainingInput() { + String malicious = + "00000000000000000000000000000000000000000000000000000000000003e8"; // length=1000 + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicBytes(malicious, 0)); + } + + @Test + public void decodeDynamicBytes_rejectsLengthCausingShiftOverflow() { + // length = 2^30, shifting left by 1 produces 2^31 (= Integer.MIN_VALUE). + String malicious = + "0000000000000000000000000000000000000000000000000000000040000000"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicBytes(malicious, 0)); + } + + @Test + public void decodeUtf8String_rejectsLengthExceedingRemainingInput() { + String malicious = + "00000000000000000000000000000000000000000000000000000000000003e8"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeUtf8String(malicious, 0)); + } + + public static class TwoStrings extends DynamicStruct { + public String a; + public String b; + + public TwoStrings(Utf8String a, Utf8String b) { + super(a, b); + this.a = a.getValue(); + this.b = b.getValue(); + } + } + + @Test + public void decodeDynamicStruct_rejectsParameterOffsetCausingMulOverflow() { + // First dynamic-parameter offset = 0x40000001 (slightly above 2^30); + // 0x40000001 * 2 + 64 overflows into a negative int. + String malicious = + "0000000000000000000000000000000000000000000000000000000040000001" + + "00000000000000000000000000000000000000000000000000000000000000c0" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "6100000000000000000000000000000000000000000000000000000000000000"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicStruct( + malicious, 0, new TypeReference() {})); + } + + @Test + public void decodeDynamicArray_rejectsElementDataOffsetCausingMulOverflow() { + // length=1; element0's dynamic head word (the data-offset pointer) = 0x40000000 + // (= 2^30, bitLength 31, so it passes the decodeUintAsInt cap). getDataOffset then + // computes `pointer * 2`, which overflows int — without the multiplyExact guard it + // would wrap to a negative offset and silently alias earlier bytes. + String malicious = + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000040000000"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicArray( + malicious, 0, new TypeReference>() {})); + } + + @Test + public void decodeDynamicArray_arrayLengthBoundRespectsOffset() { + String malicious = + "0000000000000000000000000000000000000000000000000000000000000003" // length=3 + + "0000000000000000000000000000000000000000000000000000000000000001" // elem0 + + "0000000000000000000000000000000000000000000000000000000000000002"; // elem1 (no elem2!) + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicArray( + malicious, 0, new TypeReference>() {})); + } + + /** Solidity: {@code struct TwoUints { uint256 a; uint256 b; }} (all-static, 2 slots). */ + public static class TwoUints extends StaticStruct { + public BigInteger a; + public BigInteger b; + + public TwoUints(Uint256 a, Uint256 b) { + super(a, b); + this.a = a.getValue(); + this.b = b.getValue(); + } + } + + @Test + public void decodeDynamicArray_advancesMultiSlotElementsToExactInputEnd() { + // A well-formed array of 2-slot static structs. This routes through + // advanceArrayElementOffset's static-struct site with a multi-slot stride + // (bytes32PaddedLength/32 = 2 slots per element), and the final element ends + // exactly at input.length() — locking the strict `>` boundary so a legitimate + // last element is not falsely rejected. + String legit = + "0000000000000000000000000000000000000000000000000000000000000002" // length=2 + + "0000000000000000000000000000000000000000000000000000000000000001" // elem0.a=1 + + "0000000000000000000000000000000000000000000000000000000000000002" // elem0.b=2 + + "0000000000000000000000000000000000000000000000000000000000000003" // elem1.a=3 + + "0000000000000000000000000000000000000000000000000000000000000004"; // elem1.b=4 + DynamicArray result = TypeDecoder.decodeDynamicArray( + legit, 0, new TypeReference>() {}); + assertEquals(2, result.getValue().size()); + } + + /** A nested TwoUints (128 hex chars) + trailing uint256 (64) = 192 hex chars expected. */ + public static class NestedTwoUints extends StaticStruct { + public TwoUints inner; + public BigInteger c; + + public NestedTwoUints(TwoUints inner, Uint256 c) { + super(inner, c); + this.inner = inner; + this.c = c.getValue(); + } + } + + @Test + public void decodeUintAsInt_rejectsInputShorterThanSlot() { + String malicious = "00"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeUintAsInt(malicious, 0)); + } + + @Test + public void decodeUintAsInt_rejectsOffsetPastInput() { + // Caller-supplied offset already past the end of input. + String malicious = + "0000000000000000000000000000000000000000000000000000000000000001"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeUintAsInt(malicious, 64)); + } + + @Test + public void decodeBool_rejectsInputShorterThanSlot() { + String malicious = "01"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeBool(malicious, 0)); + } + + @Test + public void decodeBool_rejectsOffsetPastInput() { + // Caller-supplied offset already past the end of input. + String malicious = + "0000000000000000000000000000000000000000000000000000000000000001"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeBool(malicious, 64)); + } + + @Test + public void decodeDynamicStruct_rejectsInputShorterThanStaticHead() { + String malicious = + "0000000000000000000000000000000000000000000000000000000000000020"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicStruct( + malicious, 0, new TypeReference() {})); + } + + public static class TwoStringsAndBytes extends DynamicStruct { + public String a; + public String b; + public byte[] c; + + public TwoStringsAndBytes(Utf8String a, Utf8String b, DynamicBytes c) { + super(a, b, c); + this.a = a.getValue(); + this.b = b.getValue(); + this.c = c.getValue(); + } + } + + /** Dynamic struct whose first field is a static uint256 — exercises the static-field path. */ + public static class StaticThenDynamic extends DynamicStruct { + public BigInteger a; + public String b; + + public StaticThenDynamic(Uint256 a, Utf8String b) { + super(a, b); + this.a = a.getValue(); + this.b = b.getValue(); + } + } + + @Test + public void decodeDynamicStruct_rejectsShortInputForStaticField() { + // Static head needs at least 64 hex chars for the first uint256 slot, + // but the input only has 60. Without a strict bounds check this slips past + // L537's `beginIndex > input.length()` guard and trips an + // ArrayIndexOutOfBoundsException inside decodeNumeric / arraycopy. + String malicious = "000000000000000000000000000000000000000000000000000000000000"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicStruct( + malicious, 0, new TypeReference() {})); + } + + @Test + public void decodeStaticStruct_rejectsInputShorterThanField() { + // TwoUints expects 128 hex chars (two 32-byte slots) but input has only 64. + // Without a bounds check the second `input.substring(64, 128)` throws SIOOBE. + String malicious = + "0000000000000000000000000000000000000000000000000000000000000001"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeStaticStruct( + malicious, 0, new TypeReference() {})); + } + + @Test + public void decodeStaticStruct_rejectsInputShorterThanNestedStruct() { + // NestedTwoUints expects 192 hex chars (nested 128 + tail 64) but only 64 provided. + // Exercises the nested-StaticStruct bounds branch. + String malicious = + "0000000000000000000000000000000000000000000000000000000000000001"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeStaticStruct( + malicious, 0, new TypeReference() {})); + } + + @Test + public void decodeDynamicStruct_rejectsParameterOffsetExceedingInputLength() { + String malicious = + // p0 byte offset = 0x80 → hex offset 320 (> input.length() = 256) + "0000000000000000000000000000000000000000000000000000000000000080" + // p1 byte offset = 0x90 → hex offset 352 (also > input.length(), + // and > p0 so parameterLength > 0) + + "0000000000000000000000000000000000000000000000000000000000000090" + // two filler slots so input.length() == 256 hex chars + + "0000000000000000000000000000000000000000000000000000000000000001" + + "6100000000000000000000000000000000000000000000000000000000000000"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicStruct( + malicious, 0, new TypeReference() {})); + } + + @Test + public void decodeStaticStruct_innerTypes_rejectsShortInput() throws Exception { + TypeReference ref = + new TypeReference( + false, + Arrays.asList( + TypeReference.makeTypeReference("uint256"), + TypeReference.makeTypeReference("uint256"))) {}; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeStaticStruct("00", 0, ref)); + } + + @Test + public void decodeDynamicStruct_innerTypes_rejectsShortHead() throws Exception { + TypeReference ref = + new TypeReference( + false, + Arrays.asList( + TypeReference.makeTypeReference("string"), + TypeReference.makeTypeReference("uint256"))) {}; + // One 64-hex-char slot: enough for the string head, nothing left for the + // uint256 static head field. + String oneSlot = + "0000000000000000000000000000000000000000000000000000000000000040"; + assertThrows( + IllegalArgumentException.class, + () -> TypeDecoder.decodeDynamicStruct(oneSlot, 0, ref)); + } + + @Test + public void decodeNumeric_rejectsInputShorterThanSlot() { + // ABI numerics are always padded to 32 bytes (64 hex chars). The guard throws + // IndexOutOfBoundsException: decodeNumeric's multi-catch swallows + // IllegalArgumentException (for reflective newInstance) and would wrap it. + String malicious = "00"; + assertThrows( + IndexOutOfBoundsException.class, + () -> TypeDecoder.decodeNumeric(malicious, Uint256.class)); + } +} diff --git a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java index c66c711b..a1bd4fc9 100644 --- a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java +++ b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java @@ -15,6 +15,7 @@ package org.tron.trident.crypto; +import com.google.common.base.Preconditions; import java.math.BigInteger; import java.security.InvalidAlgorithmParameterException; import java.security.KeyPairGenerator; @@ -121,7 +122,7 @@ public static boolean verify(final Bytes data, final Signature signature, final public static boolean verify( final Bytes data, final Signature signature, final PublicKey pub, final UnaryOperator preprocessor) { - assert preprocessor != null : "preprocessor must not be null"; + Preconditions.checkNotNull(preprocessor, "preprocessor must not be null"); return verify(preprocessor.apply(data), signature, pub); } @@ -155,10 +156,12 @@ private static ECPoint decompressKey(final BigInteger xBN, final boolean yBit) { */ private static BigInteger recoverFromSignature( final int recId, final BigInteger r, final BigInteger s, final Bytes32 dataHash) { - assert (recId >= 0); - assert (r.signum() >= 0); - assert (s.signum() >= 0); - assert (dataHash != null); + Preconditions.checkNotNull(r, "r must not be null"); + Preconditions.checkNotNull(s, "s must not be null"); + Preconditions.checkNotNull(dataHash, "dataHash must not be null"); + Preconditions.checkArgument(recId >= 0, "recId must be greater than or equal to 0"); + Preconditions.checkArgument(r.signum() >= 0, "r must be greater than or equal to 0"); + Preconditions.checkArgument(s.signum() >= 0, "s must be greater than or equal to 0"); // 1.0 For j from 0 to h (h == recId here and the loop is outside this function) // 1.1 Let x = r + jn @@ -293,8 +296,8 @@ private static boolean verifyDefault(final Bytes data, final Signature signature */ public static Bytes32 calculateECDHKeyAgreement(final PrivateKey privKey, final PublicKey theirPubKey) { - assert privKey != null : "missing private key"; - assert theirPubKey != null : "missing remote public key"; + Preconditions.checkNotNull(privKey, "missing private key"); + Preconditions.checkNotNull(theirPubKey, "missing remote public key"); final ECPrivateKeyParameters privKeyP = new ECPrivateKeyParameters(privKey.getD(), CURVE); final ECPublicKeyParameters pubKeyP = new ECPublicKeyParameters(theirPubKey.asEcPoint(), CURVE); @@ -311,12 +314,12 @@ public static class PrivateKey implements java.security.PrivateKey { private final Bytes32 encoded; private PrivateKey(final Bytes32 encoded) { - assert encoded != null; + Preconditions.checkNotNull(encoded, "encoded must not be null"); this.encoded = encoded; } public static PrivateKey create(final BigInteger key) { - assert key != null; + Preconditions.checkNotNull(key, "key must not be null"); return create(UInt256.valueOf(key).toBytes()); } @@ -325,8 +328,14 @@ public static PrivateKey create(final Bytes32 key) { } public static PrivateKey create(final String hexKey) { - assert hexKey.length() == 64; - return create(Bytes32.fromHexString(hexKey)); + Preconditions.checkNotNull(hexKey, "hexKey must not be null"); + // Tolerate the optional "0x" prefix that Bytes32.fromHexString accepts, so + // previously valid "0x"-prefixed keys keep working now that this check is + // enforced at runtime. + final String rawHex = + (hexKey.startsWith("0x") || hexKey.startsWith("0X")) ? hexKey.substring(2) : hexKey; + Preconditions.checkArgument(rawHex.length() == 64, "hexKey must be 64 hex characters long"); + return create(Bytes32.fromHexString(rawHex)); } @Override @@ -369,7 +378,15 @@ public int hashCode() { @Override public String toString() { - return encoded.toString(); + return "SECP256K1.PrivateKey{REDACTED}"; + } + + /** + * Return the privateKey String. Unless you absolutely need + * the privateKey it is better for security reasons to just use toString(). + */ + public String toStringWithPrivateKey() { + return encoded.toUnprefixedHexString(); } } @@ -395,7 +412,7 @@ public static PublicKey create(final PrivateKey privateKey) { } public static PublicKey create(final BigInteger key) { - assert key != null; + Preconditions.checkNotNull(key, "key must not be null"); return create(toBytes64(key.toByteArray())); } @@ -424,8 +441,8 @@ public static Optional recoverFromSignature(final Bytes32 dataHash, } private PublicKey(final Bytes encoded) { - assert encoded != null; - assert encoded.size() == BYTE_LENGTH; + Preconditions.checkNotNull(encoded, "encoded must not be null"); + Preconditions.checkArgument(encoded.size() == BYTE_LENGTH, "encoded byte size must be 64"); this.encoded = encoded; } @@ -487,8 +504,8 @@ public static class KeyPair { private final PublicKey publicKey; public KeyPair(final PrivateKey privateKey, final PublicKey publicKey) { - assert privateKey != null; - assert publicKey != null; + Preconditions.checkNotNull(privateKey, "privateKey must not be null"); + Preconditions.checkNotNull(publicKey, "publicKey must not be null"); this.privateKey = privateKey; this.publicKey = publicKey; } @@ -572,8 +589,8 @@ public static class Signature { * neither 27 or 28). */ public static Signature create(final BigInteger r, final BigInteger s, final byte recId) { - assert r != null; - assert s != null; + Preconditions.checkNotNull(r, "r must not be null"); + Preconditions.checkNotNull(s, "s must not be null"); checkInBounds("r", r); checkInBounds("s", s); if (recId != 0 && recId != 1) { @@ -596,7 +613,9 @@ private static void checkInBounds(final String name, final BigInteger i) { } public static Signature decode(final Bytes bytes) { - assert bytes.size() == BYTES_REQUIRED : "encoded SECP256K1 signature must be 65 bytes long"; + Preconditions.checkNotNull(bytes, "bytes must not be null"); + Preconditions.checkArgument(bytes.size() == BYTES_REQUIRED, + "encoded SECP256K1 signature must be 65 bytes long"); final BigInteger r = bytes.slice(0, 32).toUnsignedBigInteger(); final BigInteger s = bytes.slice(32, 32).toUnsignedBigInteger();