Skip to content

Commit 9362961

Browse files
authored
fix(jsonrpc): harden ABI parser bounds and revert reason decoding (#6711)
1 parent 709e1c3 commit 9362961

5 files changed

Lines changed: 591 additions & 21 deletions

File tree

framework/src/main/java/org/tron/common/logsfilter/ContractEventParser.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
package org.tron.common.logsfilter;
22

3-
import static org.tron.common.math.Maths.min;
4-
53
import java.math.BigInteger;
4+
import java.nio.charset.StandardCharsets;
65
import java.util.regex.Pattern;
76
import lombok.extern.slf4j.Slf4j;
87
import org.apache.commons.lang3.ArrayUtils;
@@ -38,9 +37,14 @@ public static String parseDataBytes(byte[] data, String typeStr, int index) {
3837
byte[] lengthBytes = subBytes(data, start, DATAWORD_UNIT_SIZE);
3938
// this length is byte count. no need X 32
4039
int length = intValueExact(lengthBytes);
40+
if (length < 0) {
41+
throw new OutputLengthException("data length:" + length);
42+
}
4143
byte[] realBytes =
4244
length > 0 ? subBytes(data, start + DATAWORD_UNIT_SIZE, length) : new byte[0];
43-
return type == Type.STRING ? new String(realBytes) : Hex.toHexString(realBytes);
45+
return type == Type.STRING
46+
? new String(realBytes, StandardCharsets.UTF_8)
47+
: Hex.toHexString(realBytes);
4448
}
4549
} catch (OutputLengthException | ArithmeticException e) {
4650
logger.debug("parseDataBytes ", e);
@@ -74,11 +78,15 @@ protected static Integer intValueExact(byte[] data) {
7478
}
7579

7680
protected static byte[] subBytes(byte[] src, int start, int length) {
77-
if (ArrayUtils.isEmpty(src) || start >= src.length || length < 0) {
78-
throw new OutputLengthException("data start:" + start + ", length:" + length);
81+
if (ArrayUtils.isEmpty(src)) {
82+
throw new OutputLengthException("source data is empty");
83+
}
84+
if (start < 0 || start >= src.length || length < 0 || length > src.length - start) {
85+
throw new OutputLengthException(
86+
"data start:" + start + ", length:" + length + ", src.length:" + src.length);
7987
}
8088
byte[] dst = new byte[length];
81-
System.arraycopy(src, start, dst, 0, min(length, src.length - start, true));
89+
System.arraycopy(src, start, dst, 0, length);
8290
return dst;
8391
}
8492

framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ public enum RequestSource {
167167
private static final String NO_BLOCK_HEADER_BY_HASH = "header for hash not found";
168168

169169
private static final String ERROR_SELECTOR = "08c379a0"; // Function selector for Error(string)
170+
private static final int REVERT_REASON_SELECTOR_LENGTH = 4;
171+
private static final int MAX_REVERT_REASON_PAYLOAD_BYTES = 4096;
170172
/**
171173
* thread pool of query section bloom store
172174
*/
@@ -483,6 +485,36 @@ private void estimateEnergy(byte[] ownerAddressByte, byte[] contractAddressByte,
483485
estimateBuilder.setResult(retBuilder);
484486
}
485487

488+
/**
489+
* Decodes an Error(string) revert reason when possible.
490+
* Returns ": reason" for a non-empty reason, otherwise "".
491+
*/
492+
static String tryDecodeRevertReason(byte[] resData) {
493+
if (resData == null || resData.length <= REVERT_REASON_SELECTOR_LENGTH) {
494+
return "";
495+
}
496+
if (!Hex.toHexString(resData, 0, REVERT_REASON_SELECTOR_LENGTH).equals(ERROR_SELECTOR)) {
497+
return "";
498+
}
499+
500+
int revertPayloadLength = resData.length - REVERT_REASON_SELECTOR_LENGTH;
501+
if (revertPayloadLength > MAX_REVERT_REASON_PAYLOAD_BYTES) {
502+
logger.debug("skip parsing oversized revert reason payload: {} bytes", revertPayloadLength);
503+
return "";
504+
}
505+
506+
try {
507+
String reason = ContractEventParser.parseDataBytes(
508+
Arrays.copyOfRange(resData, REVERT_REASON_SELECTOR_LENGTH,
509+
resData.length),
510+
"string", 0);
511+
return reason.isEmpty() ? "" : ": " + reason;
512+
} catch (RuntimeException e) {
513+
logger.debug("parse revert reason failed", e);
514+
return "";
515+
}
516+
}
517+
486518
/**
487519
* @param data Hash of the method signature and encoded parameters. for example:
488520
* getMethodSign(methodName(uint256,uint256)) || data1 || data2
@@ -526,14 +558,8 @@ private String call(byte[] ownerAddressByte, byte[] contractAddressByte, long va
526558
}
527559
result = ByteArray.toJsonHex(listBytes);
528560
} else {
529-
String errMsg = retBuilder.getMessage().toStringUtf8();
530561
byte[] resData = trxExtBuilder.getConstantResult(0).toByteArray();
531-
if (resData.length > 4 && Hex.toHexString(resData).startsWith(ERROR_SELECTOR)) {
532-
String msg = ContractEventParser
533-
.parseDataBytes(org.bouncycastle.util.Arrays.copyOfRange(resData, 4, resData.length),
534-
"string", 0);
535-
errMsg += ": " + msg;
536-
}
562+
String errMsg = retBuilder.getMessage().toStringUtf8() + tryDecodeRevertReason(resData);
537563

538564
if (resData.length > 0) {
539565
throw new JsonRpcInternalException(errMsg, ByteArray.toJsonHex(resData));
@@ -666,15 +692,8 @@ public String estimateGas(CallArguments args) throws JsonRpcInvalidRequestExcept
666692
}
667693

668694
if (trxExtBuilder.getTransaction().getRet(0).getRet().equals(code.FAILED)) {
669-
String errMsg = retBuilder.getMessage().toStringUtf8();
670-
671695
byte[] data = trxExtBuilder.getConstantResult(0).toByteArray();
672-
if (data.length > 4 && Hex.toHexString(data).startsWith(ERROR_SELECTOR)) {
673-
String msg = ContractEventParser
674-
.parseDataBytes(org.bouncycastle.util.Arrays.copyOfRange(data, 4, data.length),
675-
"string", 0);
676-
errMsg += ": " + msg;
677-
}
696+
String errMsg = retBuilder.getMessage().toStringUtf8() + tryDecodeRevertReason(data);
678697

679698
if (data.length > 0) {
680699
throw new JsonRpcInternalException(errMsg, ByteArray.toJsonHex(data));

framework/src/test/java/org/tron/common/logsfilter/EventParserTest.java

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.util.LinkedList;
66
import java.util.List;
77
import java.util.Map;
8+
import org.bouncycastle.crypto.OutputLengthException;
89
import org.bouncycastle.util.Arrays;
910
import org.junit.Assert;
1011
import org.junit.Test;
@@ -100,6 +101,91 @@ public synchronized void testEventParser() {
100101

101102
}
102103

104+
@Test
105+
public void testParseDataBytesIntegerTypes() {
106+
// uint256 = 255
107+
byte[] uintData = ByteArray.fromHexString(
108+
"00000000000000000000000000000000000000000000000000000000000000ff");
109+
Assert.assertEquals("255", ContractEventParser.parseDataBytes(uintData, "uint256", 0));
110+
111+
// int256 = -1 (two's complement 0xFF..FF is signed negative one)
112+
byte[] negIntData = ByteArray.fromHexString(
113+
"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
114+
Assert.assertEquals("-1", ContractEventParser.parseDataBytes(negIntData, "int256", 0));
115+
116+
// trcToken is classified as INT_NUMBER
117+
byte[] tokenData = ByteArray.fromHexString(
118+
"0000000000000000000000000000000000000000000000000000000000000064");
119+
Assert.assertEquals("100", ContractEventParser.parseDataBytes(tokenData, "trcToken", 0));
120+
}
121+
122+
@Test
123+
public void testParseDataBytesBool() {
124+
byte[] trueData = ByteArray.fromHexString(
125+
"0000000000000000000000000000000000000000000000000000000000000001");
126+
Assert.assertEquals("true", ContractEventParser.parseDataBytes(trueData, "bool", 0));
127+
128+
byte[] falseData = ByteArray.fromHexString(
129+
"0000000000000000000000000000000000000000000000000000000000000000");
130+
Assert.assertEquals("false", ContractEventParser.parseDataBytes(falseData, "bool", 0));
131+
}
132+
133+
@Test
134+
public void testParseDataBytesFixedBytes() {
135+
String hex = "1234567890abcdef0000000000000000000000000000000000000000000000ff";
136+
byte[] data = ByteArray.fromHexString(hex);
137+
Assert.assertEquals(hex, ContractEventParser.parseDataBytes(data, "bytes32", 0));
138+
}
139+
140+
@Test
141+
public void testParseDataBytesAddress() {
142+
Wallet.setAddressPreFixByte(ADD_PRE_FIX_BYTE_MAINNET);
143+
// last 20 bytes = ca35...733c => Base58Check = TUQPrDEJkV4ttkrL7cVv1p3mikWYfM7LWt
144+
byte[] data = ByteArray.fromHexString(
145+
"000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733c");
146+
Assert.assertEquals("TUQPrDEJkV4ttkrL7cVv1p3mikWYfM7LWt",
147+
ContractEventParser.parseDataBytes(data, "address", 0));
148+
}
149+
150+
@Test
151+
public void testParseDataBytesDynamicBytes() {
152+
// offset 0x20 | length 3 | 0x010203 padded to 32 bytes
153+
byte[] data = ByteArray.fromHexString(
154+
"0000000000000000000000000000000000000000000000000000000000000020"
155+
+ "0000000000000000000000000000000000000000000000000000000000000003"
156+
+ "0102030000000000000000000000000000000000000000000000000000000000");
157+
Assert.assertEquals("010203", ContractEventParser.parseDataBytes(data, "bytes", 0));
158+
}
159+
160+
@Test
161+
public void testParseDataBytesEmptyString() {
162+
// offset 0x20 | length 0
163+
byte[] data = ByteArray.fromHexString(
164+
"0000000000000000000000000000000000000000000000000000000000000020"
165+
+ "0000000000000000000000000000000000000000000000000000000000000000");
166+
Assert.assertEquals("", ContractEventParser.parseDataBytes(data, "string", 0));
167+
}
168+
169+
@Test
170+
public void testParseDataBytesNonEmptyString() {
171+
// "hello world" is 11 ASCII bytes (68656c6c6f20776f726c64), padded to 32 bytes.
172+
byte[] data = ByteArray.fromHexString(
173+
"0000000000000000000000000000000000000000000000000000000000000020"
174+
+ "000000000000000000000000000000000000000000000000000000000000000b"
175+
+ "68656c6c6f20776f726c64000000000000000000000000000000000000000000");
176+
Assert.assertEquals("hello world", ContractEventParser.parseDataBytes(data, "string", 0));
177+
}
178+
179+
@Test
180+
public void testParseDataBytesMultiByteUtf8String() {
181+
// "中文" UTF-8 = e4b8ad e69687 (6 bytes), padded to 32 bytes.
182+
byte[] data = ByteArray.fromHexString(
183+
"0000000000000000000000000000000000000000000000000000000000000020"
184+
+ "0000000000000000000000000000000000000000000000000000000000000006"
185+
+ "e4b8ade696870000000000000000000000000000000000000000000000000000");
186+
Assert.assertEquals("中文", ContractEventParser.parseDataBytes(data, "string", 0));
187+
}
188+
103189
@Test
104190
public void testParseRevert() {
105191
String dataHex = "08c379a0"
@@ -113,4 +199,87 @@ public void testParseRevert() {
113199
Assert.assertEquals(msg, "not enough input value");
114200

115201
}
202+
203+
@Test
204+
public void testSubBytesRejectsOversizedLength() {
205+
// Length must fit in the available source bytes. Reject instead of
206+
// truncating so oversized ABI lengths are not silently coerced.
207+
byte[] src = new byte[]{1, 2, 3};
208+
try {
209+
ContractEventParser.subBytes(src, 0, Integer.MAX_VALUE);
210+
Assert.fail("Expected OutputLengthException");
211+
} catch (OutputLengthException e) {
212+
Assert.assertTrue(e.getMessage().contains("data start:0"));
213+
Assert.assertTrue(e.getMessage().contains("length:2147483647"));
214+
Assert.assertTrue(e.getMessage().contains("src.length:3"));
215+
}
216+
}
217+
218+
@Test
219+
public void testSubBytesAcceptsExactLength() {
220+
byte[] src = new byte[]{1, 2, 3, 4};
221+
byte[] result = ContractEventParser.subBytes(src, 1, 3);
222+
Assert.assertArrayEquals(new byte[]{2, 3, 4}, result);
223+
}
224+
225+
@Test
226+
public void testSubBytesRejectsNegativeOffset() {
227+
// ABI offsets are unsigned, but BigInteger(byte[]) interprets 0xFF..FF as
228+
// -1. The guard should reject that value before System.arraycopy runs.
229+
byte[] src = new byte[]{1, 2, 3, 4};
230+
try {
231+
ContractEventParser.subBytes(src, -1, 3);
232+
Assert.fail("Expected OutputLengthException");
233+
} catch (OutputLengthException e) {
234+
Assert.assertTrue(e.getMessage().contains("data start:-1"));
235+
Assert.assertTrue(e.getMessage().contains("length:3"));
236+
Assert.assertTrue(e.getMessage().contains("src.length:4"));
237+
}
238+
}
239+
240+
@Test
241+
public void testSubBytesRejectsEmptySource() {
242+
try {
243+
ContractEventParser.subBytes(new byte[0], 0, 0);
244+
Assert.fail("Expected OutputLengthException");
245+
} catch (OutputLengthException e) {
246+
Assert.assertTrue(e.getMessage().contains("source data is empty"));
247+
}
248+
}
249+
250+
@Test(expected = UnsupportedOperationException.class)
251+
public void testParseDataBytesRejectsNegativeOffset() {
252+
// End-to-end check: an offset field of 0xFF..FF decodes to -1 and should
253+
// be rejected through the existing UnsupportedOperationException path.
254+
String dataHex = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
255+
+ "0000000000000000000000000000000000000000000000000000000000000003"
256+
+ "414243";
257+
byte[] data = ByteArray.fromHexString(dataHex);
258+
259+
ContractEventParser.parseDataBytes(data, "string", 0);
260+
}
261+
262+
@Test(expected = UnsupportedOperationException.class)
263+
public void testParseDataBytesRejectsMalformedLength() {
264+
// ABI-encoded "string" whose declared length exceeds the available payload
265+
// should be rejected via the existing UnsupportedOperationException path.
266+
String dataHex = "0000000000000000000000000000000000000000000000000000000000000020"
267+
+ "000000000000000000000000000000000000000000000000000000007fffffff"
268+
+ "414243";
269+
byte[] data = ByteArray.fromHexString(dataHex);
270+
271+
ContractEventParser.parseDataBytes(data, "string", 0);
272+
}
273+
274+
@Test(expected = UnsupportedOperationException.class)
275+
public void testParseDataBytesRejectsNegativeLength() {
276+
// ABI length is an unsigned word. If 0xFF..FF is decoded as -1, reject it
277+
// instead of treating it as an empty string/bytes payload.
278+
String dataHex = "0000000000000000000000000000000000000000000000000000000000000020"
279+
+ "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
280+
+ "414243";
281+
byte[] data = ByteArray.fromHexString(dataHex);
282+
283+
ContractEventParser.parseDataBytes(data, "string", 0);
284+
}
116285
}

0 commit comments

Comments
 (0)