diff --git a/framework/src/main/java/org/tron/core/services/http/JsonFormat.java b/framework/src/main/java/org/tron/core/services/http/JsonFormat.java index 70b59e72b4..b2b2eeec8d 100644 --- a/framework/src/main/java/org/tron/core/services/http/JsonFormat.java +++ b/framework/src/main/java/org/tron/core/services/http/JsonFormat.java @@ -57,6 +57,7 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT import org.tron.common.utils.ByteArray; import org.tron.common.utils.Commons; import org.tron.common.utils.StringUtil; +import org.tron.core.Constant; import org.tron.json.JSON; import org.tron.protos.contract.BalanceContract; @@ -291,6 +292,7 @@ public static void merge(CharSequence input, tokenizer.consume("{"); // Needs to happen when the object starts. while (!tokenizer.tryConsume("}")) { // Continue till the object is done mergeField(tokenizer, extensionRegistry, builder, selfType); + tokenizer.tryConsume(","); } // Test to make sure the tokenizer has reached the end of the stream. if (!tokenizer.atEnd()) { @@ -556,8 +558,9 @@ protected static StringBuilder toStringBuilder(Readable input) throws IOExceptio } /** - * Parse a single field from {@code tokenizer} and merge it into {@code builder}. If a ',' is - * detected after the field ends, the next field will be parsed automatically + * Parse a single field from {@code tokenizer} and merge it into {@code builder}. Exactly one + * field is consumed; the caller ({@code merge} / {@code handleObject}) consumes any trailing + * ',' and loops over the remaining fields. */ protected static void mergeField(Tokenizer tokenizer, ExtensionRegistry extensionRegistry, Message.Builder builder, @@ -628,11 +631,6 @@ protected static void mergeField(Tokenizer tokenizer, handleValue(tokenizer, extensionRegistry, builder, field, extension, unknown, selfType); } } - - if (tokenizer.tryConsume(",")) { - // Continue with the next field - mergeField(tokenizer, extensionRegistry, builder, selfType); - } } private static void handleMissingField(Tokenizer tokenizer, @@ -642,18 +640,28 @@ private static void handleMissingField(Tokenizer tokenizer, if ("{".equals(tokenizer.currentToken())) { // Message structure tokenizer.consume("{"); - do { - tokenizer.consumeIdentifier(); - handleMissingField(tokenizer, extensionRegistry, builder); - } while (tokenizer.tryConsume(",")); - tokenizer.consume("}"); + tokenizer.enterRecursion(); + try { + do { + tokenizer.consumeIdentifier(); + handleMissingField(tokenizer, extensionRegistry, builder); + } while (tokenizer.tryConsume(",")); + tokenizer.consume("}"); + } finally { + tokenizer.exitRecursion(); + } } else if ("[".equals(tokenizer.currentToken())) { // Collection tokenizer.consume("["); - do { - handleMissingField(tokenizer, extensionRegistry, builder); - } while (tokenizer.tryConsume(",")); - tokenizer.consume("]"); + tokenizer.enterRecursion(); + try { + do { + handleMissingField(tokenizer, extensionRegistry, builder); + } while (tokenizer.tryConsume(",")); + tokenizer.consume("]"); + } finally { + tokenizer.exitRecursion(); + } } else { //if (!",".equals(tokenizer.currentToken)){ // Primitive value if ("null".equals(tokenizer.currentToken())) { @@ -807,20 +815,25 @@ private static Object handleObject(Tokenizer tokenizer, } tokenizer.consume("{"); - String endToken = "}"; + tokenizer.enterRecursion(); + try { + String endToken = "}"; - while (!tokenizer.tryConsume(endToken)) { - if (tokenizer.atEnd()) { - throw tokenizer.parseException("Expected \"" + endToken + "\"."); - } - mergeField(tokenizer, extensionRegistry, subBuilder, selfType); - if (tokenizer.tryConsume(",")) { - // there are more fields in the object, so continue - continue; + while (!tokenizer.tryConsume(endToken)) { + if (tokenizer.atEnd()) { + throw tokenizer.parseException("Expected \"" + endToken + "\"."); + } + mergeField(tokenizer, extensionRegistry, subBuilder, selfType); + if (tokenizer.tryConsume(",")) { + // there are more fields in the object, so continue + continue; + } } - } - return subBuilder.build(); + return subBuilder.build(); + } finally { + tokenizer.exitRecursion(); + } } /** @@ -1290,6 +1303,18 @@ protected static class Tokenizer { // errors *after* consuming). private int previousLine = 0; private int previousColumn = 0; + private int currentDepth = 0; + + public void enterRecursion() throws ParseException { + if (currentDepth >= Constant.MAX_NESTING_DEPTH) { + throw parseException("Hit recursion limit."); + } + ++currentDepth; + } + + public void exitRecursion() { + --currentDepth; + } /** * Construct a tokenizer that parses tokens from the given text. diff --git a/framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java b/framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java index 8256875589..825b1ff2f3 100644 --- a/framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java +++ b/framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java @@ -1143,7 +1143,7 @@ private TransactionJson buildCreateSmartContractTransaction(byte[] ownerAddress, String abiStr = "{" + "\"entrys\":" + args.getAbi() + "}"; try { JsonFormat.merge(abiStr, abiBuilder, args.isVisible()); - } catch (StackOverflowError e) { + } catch (JsonFormat.ParseException | StackOverflowError e) { throw new JsonRpcInvalidParamsException("invalid abi"); } } diff --git a/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java b/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java index 870ce31766..5b2875fc24 100644 --- a/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java +++ b/framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java @@ -1461,9 +1461,8 @@ public void testBuildTransactionTransfer() { @Test public void testBuildTransactionRejectsDeeplyNestedAbi() { - // A pathological ABI with deep nesting overflows the recursive JsonFormat parser. - // buildCreateSmartContractTransaction must surface this as invalid-params (-32602), - // not let a StackOverflowError escape as a generic internal error. + // A deeply nested ABI must surface as invalid-params (-32602), not as a generic + // internal error. int depth = 200_000; StringBuilder abi = new StringBuilder("[],\"x\":"); for (int i = 0; i < depth; i++) { diff --git a/framework/src/test/java/org/tron/core/services/http/JsonFormatTest.java b/framework/src/test/java/org/tron/core/services/http/JsonFormatTest.java index a8525b0f52..e38af6ead2 100644 --- a/framework/src/test/java/org/tron/core/services/http/JsonFormatTest.java +++ b/framework/src/test/java/org/tron/core/services/http/JsonFormatTest.java @@ -17,6 +17,7 @@ import org.junit.After; import org.junit.Test; import org.mockito.Mockito; +import org.tron.core.Constant; import org.tron.protos.Protocol; public class JsonFormatTest { @@ -252,4 +253,185 @@ public void testParseInteger() throws Exception { assertTrue(cause instanceof NumberFormatException); } -} \ No newline at end of file + /* + * Compatibility-preserved behavior: these cases pass before and after this fix. + * They guard unknown-field skipping, repeated-field syntax, and accepted depth. + */ + @Test + public void testMissingFieldSkipsNestedObjectAndArray() throws Exception { + String input = "{\"zzz\":{\"a\":1,\"b\":[true,false,null,{\"c\":\"d\"}],\"e\":{\"f\":2}}," + + "\"address\":\"61646472657373\"}"; + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + + JsonFormat.merge(input, builder, false); + + assertEquals(ByteString.copyFromUtf8("address"), builder.getAddress()); + } + + @Test + public void testTrailingCommaInKnownRepeatedField() throws Exception { + Protocol.Proposal.Builder builder = Protocol.Proposal.newBuilder(); + + JsonFormat.merge("{\"approvals\":[\"00\",\"01\",]}", builder, false); + + Protocol.Proposal proposal = builder.build(); + assertEquals(2, proposal.getApprovalsCount()); + assertEquals(ByteString.copyFrom(new byte[] {0}), proposal.getApprovals(0)); + assertEquals(ByteString.copyFrom(new byte[] {1}), proposal.getApprovals(1)); + } + + @Test + public void testKnownRepeatedPrimitiveFieldRejectsNestedArray() { + Protocol.Proposal.Builder builder = Protocol.Proposal.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"approvals\":[[\"00\"]]}", builder, false)); + + assertTrue(e.getMessage().contains("Expected string.")); + } + + @Test + public void testKnownRepeatedMessageFieldRejectsNestedArray() { + Protocol.Block.Builder builder = Protocol.Block.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"transactions\":[[]]}", builder, false)); + + assertTrue(e.getMessage().contains("Expected \"{\".")); + } + + @Test + public void testMissingFieldRejectsTrailingCommaInNestedObject() { + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"zzz\":{\"a\":1,}}", builder)); + + assertTrue(e.getMessage().contains("Expected identifier")); + } + + @Test + public void testMissingFieldRejectsTrailingCommaInNestedArray() { + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge("{\"zzz\":[1,]}", builder)); + + assertTrue(e.getMessage().contains("Expected string.")); + } + + @Test + public void testNestingWithinLimit() throws Exception { + int depth = Constant.MAX_NESTING_DEPTH / 2; + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < depth; i++) { + sb.append("{\"zzz\":"); + } + sb.append("1"); + for (int i = 0; i < depth; i++) { + sb.append("}"); + } + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + JsonFormat.merge(sb.toString(), builder); + assertNotNull(builder.build()); + } + + /* + * Behavior changed by this fix: the previous parser either missed the depth guard + * or failed through recursive comma handling / stack overflow. + */ + @Test + public void testTrailingCommaInParsedObjects() throws Exception { + String input = "{\"genesisBlockId\":{\"hash\":\"00\",\"number\":1,}," + + "\"address\":\"61646472657373\",}"; + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + + JsonFormat.merge(input, builder, false); + + Protocol.HelloMessage message = builder.build(); + assertEquals(ByteString.copyFrom(new byte[] {0}), message.getGenesisBlockId().getHash()); + assertEquals(1L, message.getGenesisBlockId().getNumber()); + assertEquals(ByteString.copyFromUtf8("address"), message.getAddress()); + } + + @Test + public void testMissingFieldRejectsObjectBeyondNestingLimit() { + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge(buildUnknownNestedObject(Constant.MAX_NESTING_DEPTH + 1), builder)); + + assertTrue(e.getMessage().contains("Hit recursion limit.")); + } + + @Test + public void testMissingFieldRejectsArrayBeyondNestingLimit() { + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + + JsonFormat.ParseException e = assertThrows(JsonFormat.ParseException.class, + () -> JsonFormat.merge(buildUnknownNestedArray(Constant.MAX_NESTING_DEPTH + 1), builder)); + + assertTrue(e.getMessage().contains("Hit recursion limit.")); + } + + @Test + public void testDeeplyNestedObject() { + int depth = 100_000; + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < depth; i++) { + sb.append("{\"zzz\":"); + } + sb.append("1"); + for (int i = 0; i < depth; i++) { + sb.append("}"); + } + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + assertThrows(JsonFormat.ParseException.class, () -> JsonFormat.merge(sb.toString(), builder)); + } + + @Test + public void testDeeplyNestedArray() { + int depth = 100_000; + StringBuilder sb = new StringBuilder(); + sb.append("{\"zzz\":"); + for (int i = 0; i < depth; i++) { + sb.append("["); + } + sb.append("1"); + for (int i = 0; i < depth; i++) { + sb.append("]"); + } + sb.append("}"); + Protocol.HelloMessage.Builder builder = Protocol.HelloMessage.newBuilder(); + assertThrows(JsonFormat.ParseException.class, () -> JsonFormat.merge(sb.toString(), builder)); + } + + private String buildUnknownNestedObject(int depth) { + StringBuilder sb = new StringBuilder(); + sb.append("{"); + for (int i = 0; i < depth; i++) { + sb.append("\"zzz\":{"); + } + sb.append("\"leaf\":1"); + for (int i = 0; i < depth; i++) { + sb.append("}"); + } + sb.append("}"); + return sb.toString(); + } + + private String buildUnknownNestedArray(int depth) { + StringBuilder sb = new StringBuilder(); + sb.append("{\"zzz\":"); + for (int i = 0; i < depth; i++) { + sb.append("["); + } + sb.append("1"); + for (int i = 0; i < depth; i++) { + sb.append("]"); + } + sb.append("}"); + return sb.toString(); + } + +} diff --git a/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java b/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java new file mode 100644 index 0000000000..2a139f8a15 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java @@ -0,0 +1,49 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.common.crypto.ECKey; +import org.tron.common.utils.ByteArray; +import org.tron.core.capsule.TransactionCapsule; + +public class TriggerConstantContractServletTest extends BaseHttpTest { + + private TriggerConstantContractServlet servlet; + + @Override + protected void setUpMocks() throws Exception { + servlet = new TriggerConstantContractServlet(); + injectWallet(servlet); + } + + @Test + public void testManyFlatFieldsDoesNotOverflowStack() throws Exception { + String owner = ByteArray.toHexString(new ECKey().getAddress()); + String contract = ByteArray.toHexString(new ECKey().getAddress()); + + StringBuilder body = new StringBuilder(256 * 1024) + .append("{\"owner_address\":\"").append(owner).append('"') + .append(",\"contract_address\":\"").append(contract).append('"') + .append(",\"data\":\"00\""); + for (int i = 0; i < 20_000; i++) { + body.append(",\"x").append(i).append("\":1"); + } + body.append('}'); + + when(wallet.createTransactionCapsule(any(), any())) + .thenReturn(new TransactionCapsule(MINIMAL_TX)); + when(wallet.triggerConstantContract(any(), any(), any(), any())) + .thenReturn(MINIMAL_TX); + + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body.toString()), response); + + assertEquals(200, response.getStatus()); + verify(wallet).triggerConstantContract(any(), any(), any(), any()); + } +}