Skip to content

Commit 0871079

Browse files
authored
refactor(http): improve JsonFormat parser robustness (#6845)
Improve the hand-rolled JsonFormat parser used by HTTP wallet APIs so field-heavy and deeply nested JSON no longer overflows the request thread stack. mergeField now consumes one field per call and leaves comma iteration to the caller. JsonFormat also enforces Constant.MAX_NESTING_DEPTH for object/array descent and returns ParseException instead of allowing StackOverflowError to escape. Compatibility notes: direct JsonFormat.merge callers now reject nesting beyond 100 levels; trailing commas on known-field parse paths (top-level fields, known message fields, and known repeated fields) may be accepted, while trailing commas inside unknown nested object/array fields remain rejected; malicious deep-nesting requests now surface as parse errors instead of container-level HTTP 500s.
1 parent bb07762 commit 0871079

5 files changed

Lines changed: 287 additions & 32 deletions

File tree

framework/src/main/java/org/tron/core/services/http/JsonFormat.java

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
5757
import org.tron.common.utils.ByteArray;
5858
import org.tron.common.utils.Commons;
5959
import org.tron.common.utils.StringUtil;
60+
import org.tron.core.Constant;
6061
import org.tron.json.JSON;
6162
import org.tron.protos.contract.BalanceContract;
6263

@@ -291,6 +292,7 @@ public static void merge(CharSequence input,
291292
tokenizer.consume("{"); // Needs to happen when the object starts.
292293
while (!tokenizer.tryConsume("}")) { // Continue till the object is done
293294
mergeField(tokenizer, extensionRegistry, builder, selfType);
295+
tokenizer.tryConsume(",");
294296
}
295297
// Test to make sure the tokenizer has reached the end of the stream.
296298
if (!tokenizer.atEnd()) {
@@ -556,8 +558,9 @@ protected static StringBuilder toStringBuilder(Readable input) throws IOExceptio
556558
}
557559

558560
/**
559-
* Parse a single field from {@code tokenizer} and merge it into {@code builder}. If a ',' is
560-
* detected after the field ends, the next field will be parsed automatically
561+
* Parse a single field from {@code tokenizer} and merge it into {@code builder}. Exactly one
562+
* field is consumed; the caller ({@code merge} / {@code handleObject}) consumes any trailing
563+
* ',' and loops over the remaining fields.
561564
*/
562565
protected static void mergeField(Tokenizer tokenizer,
563566
ExtensionRegistry extensionRegistry, Message.Builder builder,
@@ -628,11 +631,6 @@ protected static void mergeField(Tokenizer tokenizer,
628631
handleValue(tokenizer, extensionRegistry, builder, field, extension, unknown, selfType);
629632
}
630633
}
631-
632-
if (tokenizer.tryConsume(",")) {
633-
// Continue with the next field
634-
mergeField(tokenizer, extensionRegistry, builder, selfType);
635-
}
636634
}
637635

638636
private static void handleMissingField(Tokenizer tokenizer,
@@ -642,18 +640,28 @@ private static void handleMissingField(Tokenizer tokenizer,
642640
if ("{".equals(tokenizer.currentToken())) {
643641
// Message structure
644642
tokenizer.consume("{");
645-
do {
646-
tokenizer.consumeIdentifier();
647-
handleMissingField(tokenizer, extensionRegistry, builder);
648-
} while (tokenizer.tryConsume(","));
649-
tokenizer.consume("}");
643+
tokenizer.enterRecursion();
644+
try {
645+
do {
646+
tokenizer.consumeIdentifier();
647+
handleMissingField(tokenizer, extensionRegistry, builder);
648+
} while (tokenizer.tryConsume(","));
649+
tokenizer.consume("}");
650+
} finally {
651+
tokenizer.exitRecursion();
652+
}
650653
} else if ("[".equals(tokenizer.currentToken())) {
651654
// Collection
652655
tokenizer.consume("[");
653-
do {
654-
handleMissingField(tokenizer, extensionRegistry, builder);
655-
} while (tokenizer.tryConsume(","));
656-
tokenizer.consume("]");
656+
tokenizer.enterRecursion();
657+
try {
658+
do {
659+
handleMissingField(tokenizer, extensionRegistry, builder);
660+
} while (tokenizer.tryConsume(","));
661+
tokenizer.consume("]");
662+
} finally {
663+
tokenizer.exitRecursion();
664+
}
657665
} else { //if (!",".equals(tokenizer.currentToken)){
658666
// Primitive value
659667
if ("null".equals(tokenizer.currentToken())) {
@@ -807,20 +815,25 @@ private static Object handleObject(Tokenizer tokenizer,
807815
}
808816

809817
tokenizer.consume("{");
810-
String endToken = "}";
818+
tokenizer.enterRecursion();
819+
try {
820+
String endToken = "}";
811821

812-
while (!tokenizer.tryConsume(endToken)) {
813-
if (tokenizer.atEnd()) {
814-
throw tokenizer.parseException("Expected \"" + endToken + "\".");
815-
}
816-
mergeField(tokenizer, extensionRegistry, subBuilder, selfType);
817-
if (tokenizer.tryConsume(",")) {
818-
// there are more fields in the object, so continue
819-
continue;
822+
while (!tokenizer.tryConsume(endToken)) {
823+
if (tokenizer.atEnd()) {
824+
throw tokenizer.parseException("Expected \"" + endToken + "\".");
825+
}
826+
mergeField(tokenizer, extensionRegistry, subBuilder, selfType);
827+
if (tokenizer.tryConsume(",")) {
828+
// there are more fields in the object, so continue
829+
continue;
830+
}
820831
}
821-
}
822832

823-
return subBuilder.build();
833+
return subBuilder.build();
834+
} finally {
835+
tokenizer.exitRecursion();
836+
}
824837
}
825838

826839
/**
@@ -1290,6 +1303,18 @@ protected static class Tokenizer {
12901303
// errors *after* consuming).
12911304
private int previousLine = 0;
12921305
private int previousColumn = 0;
1306+
private int currentDepth = 0;
1307+
1308+
public void enterRecursion() throws ParseException {
1309+
if (currentDepth >= Constant.MAX_NESTING_DEPTH) {
1310+
throw parseException("Hit recursion limit.");
1311+
}
1312+
++currentDepth;
1313+
}
1314+
1315+
public void exitRecursion() {
1316+
--currentDepth;
1317+
}
12931318

12941319
/**
12951320
* Construct a tokenizer that parses tokens from the given text.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1143,7 +1143,7 @@ private TransactionJson buildCreateSmartContractTransaction(byte[] ownerAddress,
11431143
String abiStr = "{" + "\"entrys\":" + args.getAbi() + "}";
11441144
try {
11451145
JsonFormat.merge(abiStr, abiBuilder, args.isVisible());
1146-
} catch (StackOverflowError e) {
1146+
} catch (JsonFormat.ParseException | StackOverflowError e) {
11471147
throw new JsonRpcInvalidParamsException("invalid abi");
11481148
}
11491149
}

framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,9 +1461,8 @@ public void testBuildTransactionTransfer() {
14611461

14621462
@Test
14631463
public void testBuildTransactionRejectsDeeplyNestedAbi() {
1464-
// A pathological ABI with deep nesting overflows the recursive JsonFormat parser.
1465-
// buildCreateSmartContractTransaction must surface this as invalid-params (-32602),
1466-
// not let a StackOverflowError escape as a generic internal error.
1464+
// A deeply nested ABI must surface as invalid-params (-32602), not as a generic
1465+
// internal error.
14671466
int depth = 200_000;
14681467
StringBuilder abi = new StringBuilder("[],\"x\":");
14691468
for (int i = 0; i < depth; i++) {

framework/src/test/java/org/tron/core/services/http/JsonFormatTest.java

Lines changed: 183 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.junit.After;
1818
import org.junit.Test;
1919
import org.mockito.Mockito;
20+
import org.tron.core.Constant;
2021
import org.tron.protos.Protocol;
2122

2223
public class JsonFormatTest {
@@ -252,4 +253,185 @@ public void testParseInteger() throws Exception {
252253
assertTrue(cause instanceof NumberFormatException);
253254
}
254255

255-
}
256+
/*
257+
* Compatibility-preserved behavior: these cases pass before and after this fix.
258+
* They guard unknown-field skipping, repeated-field syntax, and accepted depth.
259+
*/
260+
@Test
261+
public void testMissingFieldSkipsNestedObjectAndArray() throws Exception {
262+
String input = "{\"zzz\":{\"a\":1,\"b\":[true,false,null,{\"c\":\"d\"}],\"e\":{\"f\":2}},"
263+
+ "\"address\":\"61646472657373\"}";
264+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
265+
266+
JsonFormat.merge(input, builder, false);
267+
268+
assertEquals(ByteString.copyFromUtf8("address"), builder.getAddress());
269+
}
270+
271+
@Test
272+
public void testTrailingCommaInKnownRepeatedField() throws Exception {
273+
Protocol.Proposal.Builder builder = Protocol.Proposal.newBuilder();
274+
275+
JsonFormat.merge("{\"approvals\":[\"00\",\"01\",]}", builder, false);
276+
277+
Protocol.Proposal proposal = builder.build();
278+
assertEquals(2, proposal.getApprovalsCount());
279+
assertEquals(ByteString.copyFrom(new byte[] {0}), proposal.getApprovals(0));
280+
assertEquals(ByteString.copyFrom(new byte[] {1}), proposal.getApprovals(1));
281+
}
282+
283+
@Test
284+
public void testKnownRepeatedPrimitiveFieldRejectsNestedArray() {
285+
Protocol.Proposal.Builder builder = Protocol.Proposal.newBuilder();
286+
287+
JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class,
288+
() -> JsonFormat.merge("{\"approvals\":[[\"00\"]]}", builder, false));
289+
290+
assertTrue(e.getMessage().contains("Expected string."));
291+
}
292+
293+
@Test
294+
public void testKnownRepeatedMessageFieldRejectsNestedArray() {
295+
Protocol.Block.Builder builder = Protocol.Block.newBuilder();
296+
297+
JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class,
298+
() -> JsonFormat.merge("{\"transactions\":[[]]}", builder, false));
299+
300+
assertTrue(e.getMessage().contains("Expected \"{\"."));
301+
}
302+
303+
@Test
304+
public void testMissingFieldRejectsTrailingCommaInNestedObject() {
305+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
306+
307+
JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class,
308+
() -> JsonFormat.merge("{\"zzz\":{\"a\":1,}}", builder));
309+
310+
assertTrue(e.getMessage().contains("Expected identifier"));
311+
}
312+
313+
@Test
314+
public void testMissingFieldRejectsTrailingCommaInNestedArray() {
315+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
316+
317+
JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class,
318+
() -> JsonFormat.merge("{\"zzz\":[1,]}", builder));
319+
320+
assertTrue(e.getMessage().contains("Expected string."));
321+
}
322+
323+
@Test
324+
public void testNestingWithinLimit() throws Exception {
325+
int depth = Constant.MAX_NESTING_DEPTH / 2;
326+
StringBuilder sb = new StringBuilder();
327+
for (int i = 0; i < depth; i++) {
328+
sb.append("{\"zzz\":");
329+
}
330+
sb.append("1");
331+
for (int i = 0; i < depth; i++) {
332+
sb.append("}");
333+
}
334+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
335+
JsonFormat.merge(sb.toString(), builder);
336+
assertNotNull(builder.build());
337+
}
338+
339+
/*
340+
* Behavior changed by this fix: the previous parser either missed the depth guard
341+
* or failed through recursive comma handling / stack overflow.
342+
*/
343+
@Test
344+
public void testTrailingCommaInParsedObjects() throws Exception {
345+
String input = "{\"genesisBlockId\":{\"hash\":\"00\",\"number\":1,},"
346+
+ "\"address\":\"61646472657373\",}";
347+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
348+
349+
JsonFormat.merge(input, builder, false);
350+
351+
Protocol.HelloMessage message = builder.build();
352+
assertEquals(ByteString.copyFrom(new byte[] {0}), message.getGenesisBlockId().getHash());
353+
assertEquals(1L, message.getGenesisBlockId().getNumber());
354+
assertEquals(ByteString.copyFromUtf8("address"), message.getAddress());
355+
}
356+
357+
@Test
358+
public void testMissingFieldRejectsObjectBeyondNestingLimit() {
359+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
360+
361+
JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class,
362+
() -> JsonFormat.merge(buildUnknownNestedObject(Constant.MAX_NESTING_DEPTH + 1), builder));
363+
364+
assertTrue(e.getMessage().contains("Hit recursion limit."));
365+
}
366+
367+
@Test
368+
public void testMissingFieldRejectsArrayBeyondNestingLimit() {
369+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
370+
371+
JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class,
372+
() -> JsonFormat.merge(buildUnknownNestedArray(Constant.MAX_NESTING_DEPTH + 1), builder));
373+
374+
assertTrue(e.getMessage().contains("Hit recursion limit."));
375+
}
376+
377+
@Test
378+
public void testDeeplyNestedObject() {
379+
int depth = 100_000;
380+
StringBuilder sb = new StringBuilder();
381+
for (int i = 0; i < depth; i++) {
382+
sb.append("{\"zzz\":");
383+
}
384+
sb.append("1");
385+
for (int i = 0; i < depth; i++) {
386+
sb.append("}");
387+
}
388+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
389+
assertThrows(JsonFormat.ParseException.class, () -> JsonFormat.merge(sb.toString(), builder));
390+
}
391+
392+
@Test
393+
public void testDeeplyNestedArray() {
394+
int depth = 100_000;
395+
StringBuilder sb = new StringBuilder();
396+
sb.append("{\"zzz\":");
397+
for (int i = 0; i < depth; i++) {
398+
sb.append("[");
399+
}
400+
sb.append("1");
401+
for (int i = 0; i < depth; i++) {
402+
sb.append("]");
403+
}
404+
sb.append("}");
405+
Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder();
406+
assertThrows(JsonFormat.ParseException.class, () -> JsonFormat.merge(sb.toString(), builder));
407+
}
408+
409+
private String buildUnknownNestedObject(int depth) {
410+
StringBuilder sb = new StringBuilder();
411+
sb.append("{");
412+
for (int i = 0; i < depth; i++) {
413+
sb.append("\"zzz\":{");
414+
}
415+
sb.append("\"leaf\":1");
416+
for (int i = 0; i < depth; i++) {
417+
sb.append("}");
418+
}
419+
sb.append("}");
420+
return sb.toString();
421+
}
422+
423+
private String buildUnknownNestedArray(int depth) {
424+
StringBuilder sb = new StringBuilder();
425+
sb.append("{\"zzz\":");
426+
for (int i = 0; i < depth; i++) {
427+
sb.append("[");
428+
}
429+
sb.append("1");
430+
for (int i = 0; i < depth; i++) {
431+
sb.append("]");
432+
}
433+
sb.append("}");
434+
return sb.toString();
435+
}
436+
437+
}

0 commit comments

Comments
 (0)