From 815252963c1e8ff9a8e93429fcc2aa13164c90ba Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 25 Aug 2026 19:06:09 +0800 Subject: [PATCH 01/14] fix(api): reject a contract query for a malformed address getContract and getContractInfo answer null for an address that is not well formed, before the account store is read. --- .../src/main/java/org/tron/core/Wallet.java | 6 + .../WalletContractAddressValidationTest.java | 121 ++++++++++++++++++ .../java/org/tron/core/WalletMockTest.java | 8 +- 3 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 84fb41bf9f0..936ad12337d 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -3203,6 +3203,9 @@ public Transaction callConstantContract(TransactionCapsule trxCap, public SmartContract getContract(GrpcAPI.BytesMessage bytesMessage) { byte[] address = bytesMessage.getValue().toByteArray(); + if (!DecodeUtil.addressValid(address)) { + return null; + } AccountCapsule accountCapsule = chainBaseManager.getAccountStore().get(address); if (accountCapsule == null) { logger.warn( @@ -3232,6 +3235,9 @@ public SmartContract getContract(GrpcAPI.BytesMessage bytesMessage) { */ public SmartContractDataWrapper getContractInfo(GrpcAPI.BytesMessage bytesMessage) { byte[] address = bytesMessage.getValue().toByteArray(); + if (!DecodeUtil.addressValid(address)) { + return null; + } AccountCapsule accountCapsule = chainBaseManager.getAccountStore().get(address); if (accountCapsule == null) { logger.warn( diff --git a/framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java b/framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java new file mode 100644 index 00000000000..9063bc7a0e0 --- /dev/null +++ b/framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java @@ -0,0 +1,121 @@ +package org.tron.core; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.AdditionalMatchers.aryEq; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.google.protobuf.ByteString; +import java.lang.reflect.Field; +import java.util.Arrays; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI.BytesMessage; +import org.tron.common.utils.Base58; +import org.tron.core.capsule.AccountCapsule; +import org.tron.core.capsule.ContractCapsule; +import org.tron.core.store.AbiStore; +import org.tron.core.store.AccountStore; +import org.tron.core.store.ContractStore; +import org.tron.protos.contract.SmartContractOuterClass.SmartContract; + +public class WalletContractAddressValidationTest { + + // the stores are re-mocked per case, but the wallet itself carries no state these tests read; + // constructing one generates a keypair, so it is built once rather than once per address + private static Wallet wallet; + private static Field chainBaseManagerField; + + @BeforeClass + public static void setUpClass() throws Exception { + wallet = new Wallet(); + chainBaseManagerField = Wallet.class.getDeclaredField("chainBaseManager"); + chainBaseManagerField.setAccessible(true); + } + + /** + * Installs a chain base manager whose account store finds nothing, and hands back that store so + * the caller can assert whether it was ever consulted. + */ + private static AccountStore installMissingAccount() throws Exception { + ChainBaseManager chainBaseManager = mock(ChainBaseManager.class); + AccountStore accountStore = mock(AccountStore.class); + when(accountStore.get(any(byte[].class))).thenReturn((AccountCapsule) null); + when(chainBaseManager.getAccountStore()).thenReturn(accountStore); + chainBaseManagerField.set(wallet, chainBaseManager); + return accountStore; + } + + private static BytesMessage bytes(byte[] raw) { + return BytesMessage.newBuilder().setValue(ByteString.copyFrom(raw)).build(); + } + + private static byte[] canonicalAddress() { + byte[] address = new byte[21]; + Arrays.fill(address, (byte) 1); + address[0] = Wallet.getAddressPreFixByte(); + return address; + } + + @Test + public void invalidAddressesAreRejectedBeforeStorageOrBase58() throws Exception { + byte[][] invalidAddresses = { + new byte[0], + new byte[20], + new byte[22], + new byte[32 * 1024], + canonicalAddress() + }; + invalidAddresses[4][0] = (byte) (Wallet.getAddressPreFixByte() + 1); + + for (byte[] invalidAddress : invalidAddresses) { + AccountStore accountStore = installMissingAccount(); + try (MockedStatic base58 = mockStatic(Base58.class)) { + assertNull(wallet.getContract(bytes(invalidAddress))); + assertNull(wallet.getContractInfo(bytes(invalidAddress))); + verifyNoInteractions(accountStore); + base58.verifyNoInteractions(); + } + } + } + + @Test + public void canonicalMissingAddressRetainsNoResultBehavior() throws Exception { + AccountStore accountStore = installMissingAccount(); + byte[] address = canonicalAddress(); + + assertNull(wallet.getContract(bytes(address))); + assertNull(wallet.getContractInfo(bytes(address))); + + verify(accountStore, times(2)).get(aryEq(address)); + } + + @Test + public void canonicalExistingAddressRetainsContractResult() throws Exception { + ChainBaseManager chainBaseManager = mock(ChainBaseManager.class); + AccountStore accountStore = mock(AccountStore.class); + ContractStore contractStore = mock(ContractStore.class); + AbiStore abiStore = mock(AbiStore.class); + ContractCapsule contractCapsule = mock(ContractCapsule.class); + SmartContract contract = SmartContract.newBuilder().setName("existing").build(); + byte[] address = canonicalAddress(); + + when(chainBaseManager.getAccountStore()).thenReturn(accountStore); + when(chainBaseManager.getContractStore()).thenReturn(contractStore); + when(chainBaseManager.getAbiStore()).thenReturn(abiStore); + when(accountStore.get(any(byte[].class))).thenReturn(mock(AccountCapsule.class)); + when(contractStore.get(any(byte[].class))).thenReturn(contractCapsule); + when(contractCapsule.getInstance()).thenReturn(contract); + when(abiStore.get(any(byte[].class))).thenReturn(null); + chainBaseManagerField.set(wallet, chainBaseManager); + + assertSame(contract, wallet.getContract(bytes(address))); + } +} diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index 2f4c08d8f9f..7874af854a6 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -1399,8 +1399,10 @@ public void testBuildShieldedTRC20Input() throws Exception { @Test public void testGetContractInfo() throws Exception { Wallet wallet = new Wallet(); + byte[] address = new byte[21]; + address[0] = Wallet.getAddressPreFixByte(); GrpcAPI.BytesMessage bytesMessage = GrpcAPI.BytesMessage.newBuilder() - .setValue(ByteString.copyFrom("test".getBytes())) + .setValue(ByteString.copyFrom(address)) .build(); ChainBaseManager chainBaseManagerMock = mock(ChainBaseManager.class); @@ -1419,8 +1421,10 @@ public void testGetContractInfo() throws Exception { @Test public void testGetContractInfo1() throws Exception { Wallet wallet = new Wallet(); + byte[] address = new byte[21]; + address[0] = Wallet.getAddressPreFixByte(); GrpcAPI.BytesMessage bytesMessage = GrpcAPI.BytesMessage.newBuilder() - .setValue(ByteString.copyFrom("test".getBytes())) + .setValue(ByteString.copyFrom(address)) .build(); ChainBaseManager chainBaseManagerMock = mock(ChainBaseManager.class); From 3a49b86bbc56f42d4ecb47ecdf16af54b8b4d573 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 25 Aug 2026 19:07:21 +0800 Subject: [PATCH 02/14] fix(api): bound json numbers before conversion A string or number longer than 64 characters is rejected before BigDecimal sees it, and Permission_id must convert identically through both coercion paths, so a value one path silently reinterprets no longer gets through. --- .../org/tron/core/services/http/Util.java | 50 ++++++- .../tron/core/services/http/BaseHttpTest.java | 16 ++- .../http/GetExchangeByIdServletTest.java | 15 +++ .../http/JsonLongValueValidationTest.java | 97 ++++++++++++++ .../services/http/TransferServletTest.java | 99 ++++++++++++++ .../org/tron/core/services/http/UtilTest.java | 123 +++++++++++++++++- 6 files changed, 392 insertions(+), 8 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index b4902a0d9d0..033b40e2761 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -74,6 +74,9 @@ public class Util { "'events' field is deprecated and no longer supported"; public static final String PERMISSION_ID = "Permission_id"; + private static final String INVALID_PERMISSION_ID = + "invalid " + PERMISSION_ID + ": expect a 32-bit integer"; + private static final int MAX_JSON_INTEGER_VALUE_LENGTH = 64; public static final String VISIBLE = "visible"; public static final String INT64_AS_STRING_PARAM = "int64_as_string"; public static final String TRANSACTION = "transaction"; @@ -441,14 +444,48 @@ public static String getHexString(final String string) { return ByteArray.toHexString(ByteString.copyFromUtf8(string).toByteArray()); } + /** + * Rejects a json value that cannot be a bounded-length number, before anything converts it. A + * container is not a number and reaches the conversion as its full serialized form, so it has to + * be turned away by type rather than by length. An absent value is left to the caller, which is + * what distinguishes an optional key from a required one. + * + * @throws InvalidParameterException if the value is not a number, or is longer than the bound + */ + private static void checkJsonNumberValue(Object rawValue, String message) { + if (rawValue == null) { + return; + } + if (!(rawValue instanceof String || rawValue instanceof Number) + || rawValue.toString().length() > MAX_JSON_INTEGER_VALUE_LENGTH) { + throw new InvalidParameterException(message); + } + } + public static Transaction setTransactionPermissionId(JSONObject jsonObject, Transaction transaction) { - if (jsonObject.containsKey(PERMISSION_ID)) { - int permissionId = jsonObject.getInteger(PERMISSION_ID); - return setTransactionPermissionId(permissionId, transaction); + if (!jsonObject.containsKey(PERMISSION_ID)) { + return transaction; } - - return transaction; + int permissionId; + try { + Object rawValue = jsonObject.get(PERMISSION_ID); + checkJsonNumberValue(rawValue, INVALID_PERMISSION_ID); + BigDecimal value = jsonObject.getBigDecimal(PERMISSION_ID); + if (value == null) { + throw new InvalidParameterException(INVALID_PERMISSION_ID); + } + // Preserve getInteger's legacy string syntax, but require it to match the exact conversion. + int exact = value.intValueExact(); + Integer legacy = jsonObject.getInteger(PERMISSION_ID); + if (legacy == null || legacy != exact) { + throw new InvalidParameterException(INVALID_PERMISSION_ID); + } + permissionId = exact; + } catch (NumberFormatException | ArithmeticException | JSONException e) { + throw new InvalidParameterException(INVALID_PERMISSION_ID); + } + return setTransactionPermissionId(permissionId, transaction); } public static Transaction setTransactionPermissionId(int permissionId, Transaction transaction) { @@ -515,6 +552,9 @@ public static long getJsonLongValue(final JSONObject jsonObject, final String ke } public static long getJsonLongValue(JSONObject jsonObject, String key, boolean required) { + Object rawValue = jsonObject.get(key); + checkJsonNumberValue(rawValue, "invalid key [" + key + "]: expect a number of at most " + + MAX_JSON_INTEGER_VALUE_LENGTH + " characters"); BigDecimal bigDecimal = jsonObject.getBigDecimal(key); if (required && bigDecimal == null) { throw new InvalidParameterException("key [" + key + "] does not exist"); diff --git a/framework/src/test/java/org/tron/core/services/http/BaseHttpTest.java b/framework/src/test/java/org/tron/core/services/http/BaseHttpTest.java index 47710a8ca93..f4a4f507f97 100644 --- a/framework/src/test/java/org/tron/core/services/http/BaseHttpTest.java +++ b/framework/src/test/java/org/tron/core/services/http/BaseHttpTest.java @@ -4,6 +4,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.google.protobuf.Any; import com.google.protobuf.ByteString; import java.lang.reflect.Field; import javax.servlet.http.HttpServlet; @@ -20,6 +21,8 @@ import org.tron.core.Wallet; import org.tron.core.config.args.Args; import org.tron.protos.Protocol.Transaction; +import org.tron.protos.Protocol.Transaction.Contract.ContractType; +import org.tron.protos.contract.AccountContract.AccountCreateContract; /** * Base class for HTTP servlet unit tests. @@ -32,8 +35,19 @@ */ public abstract class BaseHttpTest { + /** + * A stand-in transaction for the mocked wallet. The type is the one an all-default Contract + * already declared (enum 0); what it did not carry was a matching payload, and + * Util.printTransactionToJSON drops a contract whose Any does not match its declared type -- + * silently, and only once TransactionFactory knows that type, which depends on whether any + * actuator has been constructed elsewhere in the jvm. Packing the payload the way the node's + * own builders do makes the rendering independent of that global state. + */ protected static final Transaction MINIMAL_TX = Transaction.newBuilder() - .setRawData(Transaction.raw.newBuilder().addContract(Transaction.Contract.newBuilder())) + .setRawData(Transaction.raw.newBuilder().addContract( + Transaction.Contract.newBuilder() + .setType(ContractType.AccountCreateContract) + .setParameter(Any.pack(AccountCreateContract.getDefaultInstance())))) .build(); @Mock diff --git a/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java b/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java index f67072e9856..51cfcbda108 100644 --- a/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java @@ -1,10 +1,12 @@ package org.tron.core.services.http; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.google.protobuf.ByteString; @@ -49,4 +51,17 @@ public void testGet() throws Exception { assertEquals(200, response.getStatus()); assertTrue(response.getContentAsString().contains("exchange_id")); } + + @Test + public void oversizedQuotedIdIsRejectedBeforeWalletCall() throws Exception { + String oversized = "99999999999999999999999999999999999999999999999999999999999999999"; + MockHttpServletResponse response = newResponse(); + + servlet.doPost(postRequest("{\"id\":\"" + oversized + "\"}"), response); + + verifyNoInteractions(wallet); + assertTrue(response.getContentAsString().contains("id")); + assertTrue(response.getContentAsString().contains("64 characters")); + assertFalse(response.getContentAsString().contains(oversized)); + } } diff --git a/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java b/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java new file mode 100644 index 00000000000..09211d99e27 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java @@ -0,0 +1,97 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.security.InvalidParameterException; +import org.apache.commons.lang3.StringUtils; +import org.junit.Test; +import org.tron.json.JSONObject; + +public class JsonLongValueValidationTest { + + private static JSONObject value(String value) { + return JSONObject.parseObject("{\"id\":\"" + value + "\"}"); + } + + /** The same value submitted unquoted, which reaches the node as a number rather than a string. */ + private static JSONObject unquoted(String value) { + return JSONObject.parseObject("{\"id\":" + value + "}"); + } + + @Test + public void stringLengthBoundaryIsEnforcedBeforeDecimalConversion() { + assertEquals(0L, Util.getJsonLongValue(value(StringUtils.repeat('0', 64)), "id", true)); + + String oversized = StringUtils.repeat('9', 65); + InvalidParameterException exception = assertThrows(InvalidParameterException.class, + () -> Util.getJsonLongValue(value(oversized), "id", true)); + assertTrue(exception.getMessage().contains("id")); + assertFalse(exception.getMessage().contains(oversized)); + } + + /** An unquoted token reaches this check as an already-normalized Number. */ + @Test + public void unquotedNumberNormalizedTextLengthBoundaryIsEnforced() { + // json forbids leading zeros, so the accepted case is shown by the failure it reaches + // instead: at 64 characters the bound is passed and the exact conversion is what rejects it + assertThrows(ArithmeticException.class, + () -> Util.getJsonLongValue(unquoted(StringUtils.repeat('9', 64)), "id", true)); + + String oversized = StringUtils.repeat('9', 65); + InvalidParameterException exception = assertThrows(InvalidParameterException.class, + () -> Util.getJsonLongValue(unquoted(oversized), "id", true)); + assertTrue(exception.getMessage().contains("id")); + assertFalse(exception.getMessage().contains(oversized)); + } + + @Test + public void exactLongRepresentationsRemainCompatible() { + assertEquals(Long.MIN_VALUE, + Util.getJsonLongValue(value(Long.toString(Long.MIN_VALUE)), "id", true)); + assertEquals(Long.MAX_VALUE, + Util.getJsonLongValue(value(Long.toString(Long.MAX_VALUE)), "id", true)); + assertEquals(0L, Util.getJsonLongValue(value("0"), "id", true)); + assertEquals(1L, Util.getJsonLongValue(value("1.0"), "id", true)); + assertEquals(100L, Util.getJsonLongValue(value("1e2"), "id", true)); + } + + @Test + public void existingExactConversionFailuresRemainInEffect() { + assertThrows(ArithmeticException.class, + () -> Util.getJsonLongValue(value("1.5"), "id", true)); + assertThrows(ArithmeticException.class, + () -> Util.getJsonLongValue(value("9223372036854775808"), "id", true)); + } + + /** + * A container is not a number. It used to skip the length bound altogether and reach the + * conversion as its full serialized form, bounded only by the shim's 65535-character fallback. + */ + @Test + public void nonNumericTypesAreRejectedBeforeConversion() { + String digits = StringUtils.repeat('9', 128); + String[] rawJsonValues = {"[" + digits + "]", "{\"a\":" + digits + "}", "true"}; + for (String rawJsonValue : rawJsonValues) { + JSONObject jsonObject = JSONObject.parseObject("{\"id\":" + rawJsonValue + "}"); + InvalidParameterException exception = assertThrows(InvalidParameterException.class, + () -> Util.getJsonLongValue(jsonObject, "id", true)); + assertTrue(exception.getMessage().contains("id")); + assertFalse(exception.getMessage().contains(digits)); + } + } + + /** An absent optional key must stay distinguishable from one the node refuses. */ + @Test + public void optionalMissingValueStillDefaultsToZero() { + assertEquals(0L, Util.getJsonLongValue(new JSONObject(), "id", false)); + } + + @Test + public void requiredMissingValueStillFails() { + assertThrows(InvalidParameterException.class, + () -> Util.getJsonLongValue(new JSONObject(), "id", true)); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java b/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java index b04c6255dac..67863a53f63 100644 --- a/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java @@ -1,5 +1,8 @@ package org.tron.core.services.http; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -12,6 +15,7 @@ import org.tron.common.crypto.ECKey; import org.tron.common.utils.ByteArray; import org.tron.core.capsule.TransactionCapsule; +import org.tron.json.JSONObject; import org.tron.protos.Protocol; import org.tron.protos.contract.BalanceContract; @@ -52,4 +56,99 @@ && addressEquals(((BalanceContract.TransferContract) c) eq(Protocol.Transaction.Contract.ContractType.TransferContract)); assertTransactionResponse(response); } + + private String transferJson(String permissionIdJson) { + return "{" + + "\"owner_address\": \"" + ownerAddr + "\"," + + "\"to_address\": \"" + toAddr + "\"," + + "\"amount\": 100," + + "\"Permission_id\": " + permissionIdJson + + "}"; + } + + private MockHttpServletResponse post(String permissionIdJson) throws Exception { + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(transferJson(permissionIdJson)), response); + return response; + } + + @Test + public void testPermissionIdReachesTheBuiltTransaction() throws Exception { + MockHttpServletResponse response = post("2"); + + assertTransactionResponse(response); + JSONObject contract = JSONObject.parseObject(response.getContentAsString()) + .getJSONObject("raw_data").getJSONArray("contract").getJSONObject(0); + assertEquals(2, contract.getIntValue("Permission_id")); + } + + /** + * A fraction never reaches Util.setTransactionPermissionId. Permission_id is not a + * TransferContract field, so JsonFormat.merge skips it through handleMissingField, which + * accepts only integers, booleans, strings and null -- lookingAtInteger() sees the leading + * digit, hands "1.9" to consumeInt64() and that fails first. Kept as a guard that the + * endpoint rejects it whichever layer does the rejecting. + */ + @Test + public void testFractionalPermissionIdIsRejected() throws Exception { + String content = post("1.9").getContentAsString(); + + assertTrue("must report an error", content.contains("\"Error\"")); + assertFalse("must not hand back a transaction", content.contains("txID")); + assertFalse(content.contains("\"raw_data\"")); + } + + /** + * The reachable case: 2^32+1 is a valid int64, so it clears JsonFormat.merge and lands in + * Util.setTransactionPermissionId, where BigDecimal.intValue() used to wrap it to 1 -- + * handing back a transaction built against permission 1, which the caller never asked for. + */ + @Test + public void testOutOfRangePermissionIdIsRejected() throws Exception { + String content = post("4294967297").getContentAsString(); + + assertTrue("must report an error", content.contains("\"Error\"")); + assertFalse("must not hand back a transaction built against permission 1", + content.contains("txID")); + assertFalse("must not echo the submitted value", content.contains("4294967297")); + } + + @Test + public void testQuotedScientificPermissionIdIsRejected() throws Exception { + String content = post("\"1e2\"").getContentAsString(); + + assertTrue("must report an error", content.contains("\"Error\"")); + assertFalse("must not hand back a transaction", content.contains("txID")); + assertFalse(content.contains("\"raw_data\"")); + } + + /** + * An explicit null used to reach getInteger(), whose null return blew up on unboxing with a + * bare NullPointerException. It stays an error -- writing the key states intent, so a null + * there is a caller-side defect. Omitting the key is how a caller asks for the default. + */ + @Test + public void testExplicitNullPermissionIdIsRejected() throws Exception { + String content = post("null").getContentAsString(); + + assertTrue("must report an error", content.contains("\"Error\"")); + assertFalse("must not hand back a transaction", content.contains("txID")); + assertFalse("must not surface a NullPointerException", + content.contains("NullPointerException")); + } + + @Test + public void testOmittedPermissionIdBuildsTransactionWithoutPermission() throws Exception { + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest("{" + + "\"owner_address\": \"" + ownerAddr + "\"," + + "\"to_address\": \"" + toAddr + "\"," + + "\"amount\": 100" + + "}"), response); + + assertTransactionResponse(response); + JSONObject contract = JSONObject.parseObject(response.getContentAsString()) + .getJSONObject("raw_data").getJSONArray("contract").getJSONObject(0); + assertFalse(contract.containsKey("Permission_id")); + } } diff --git a/framework/src/test/java/org/tron/core/services/http/UtilTest.java b/framework/src/test/java/org/tron/core/services/http/UtilTest.java index c619fd0de54..d31a43afe37 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilTest.java @@ -1,7 +1,10 @@ package org.tron.core.services.http; import com.google.protobuf.ByteString; +import java.security.InvalidParameterException; +import java.util.Arrays; import javax.annotation.Resource; +import org.apache.commons.lang3.StringUtils; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -236,7 +239,7 @@ public void testPackCreateSmartContractOmitsNullAbiOutputs() throws Exception { Assert.assertEquals(0, contract.getNewContract().getAbi().getEntrys(0).getOutputsCount()); } - private Transaction buildTooManySigsTransaction() { + private Transaction buildTransferTransaction() { String strTransaction = "{\n" + " \"visible\": false,\n" + " \"txID\": \"fc33817936b06e50d4b6f1797e62f52d69af6c0da580a607241a9c03a48e390e\",\n" @@ -264,7 +267,11 @@ private Transaction buildTooManySigsTransaction() { + "0a1541c076305e35aea1fe45a772fcaaab8a36e87bdb551215415624c12e308b03a1a6b21d9b86e3942fac1a" + "b92b180a70b2ccb8ea8930\"\n" + "}"; - Transaction transaction = Util.packTransaction(strTransaction, false); + return Util.packTransaction(strTransaction, false); + } + + private Transaction buildTooManySigsTransaction() { + Transaction transaction = buildTransferTransaction(); int totalSignNum = dbManager.getDynamicPropertiesStore().getTotalSignNum(); ByteString dummySig = ByteString.copyFrom(new byte[65]); Transaction.Builder builder = transaction.toBuilder(); @@ -301,4 +308,116 @@ public void testPrintSignWeightTooManySigsHttpPath() { Assert.assertTrue(jsonObject.getJSONObject("result").getString("message") .contains("too many signatures")); } + + /** + * A container is not a number. It used to skip the length bound altogether and reach the + * conversion as its full serialized form, bounded only by the shim's 65535-character fallback. + */ + @Test + public void testPermissionIdRejectsNonNumericTypes() { + assertRejected("[" + StringUtils.repeat('9', 128) + "]"); + assertRejected("{\"a\":" + StringUtils.repeat('9', 128) + "}"); + assertRejected("true"); + } + + private Transaction applyPermissionId(String rawJsonValue) { + JSONObject jsonObject = + JSONObject.parseObject("{\"" + Util.PERMISSION_ID + "\":" + rawJsonValue + "}"); + return Util.setTransactionPermissionId(jsonObject, buildTransferTransaction()); + } + + private int permissionIdOf(Transaction transaction) { + return transaction.getRawData().getContract(0).getPermissionId(); + } + + @Test + public void testPermissionIdAcceptsLegacyExactIntegerRepresentations() { + Assert.assertEquals(2, permissionIdOf(applyPermissionId("2"))); + Assert.assertEquals(2, permissionIdOf(applyPermissionId("\"2\""))); + Assert.assertEquals(1, permissionIdOf(applyPermissionId("1.0"))); + Assert.assertEquals(100, permissionIdOf(applyPermissionId("1e2"))); + Assert.assertEquals(1, permissionIdOf(applyPermissionId("\"1.0\""))); + Assert.assertEquals(1, permissionIdOf(applyPermissionId("\"1.\""))); + Assert.assertEquals(1000, permissionIdOf(applyPermissionId("\"1,000\""))); + } + + @Test + public void testPermissionIdAcceptsMaximumLengthNumericString() { + char[] digits = new char[64]; + Arrays.fill(digits, '0'); + digits[digits.length - 1] = '2'; + + Assert.assertEquals(2, permissionIdOf(applyPermissionId("\"" + new String(digits) + "\""))); + } + + /** + * One character past the bound is refused, and so is a value far beyond it: the field is + * bounded by its own length rather than by whatever the numeric conversion happens to survive. + */ + @Test + public void testPermissionIdRejectsStringsPastTheLengthBoundary() { + assertRejected("\"" + StringUtils.repeat('0', 64) + "2\""); + assertRejected("\"" + StringUtils.repeat('9', 10_000) + "\""); + } + + /** Quoting must not decide which regime applies: the same digits unquoted are refused too. */ + @Test + public void testPermissionIdRejectsUnquotedNumbersPastTheLengthBoundary() { + assertRejected("9" + StringUtils.repeat('9', 64)); + } + + private void assertRejected(String rawJsonValue) { + InvalidParameterException e = Assert.assertThrows(InvalidParameterException.class, + () -> applyPermissionId(rawJsonValue)); + Assert.assertTrue(e.getMessage().contains(Util.PERMISSION_ID)); + Assert.assertFalse("the message must not echo the submitted value", + e.getMessage().contains(rawJsonValue)); + } + + @Test + public void testPermissionIdRejectsFraction() { + assertRejected("1.9"); + assertRejected("2.999"); + } + + @Test + public void testPermissionIdRejectsNonLegacyNumericStrings() { + assertRejected("\"1e2\""); + assertRejected("\"1E2\""); + assertRejected("\".0\""); + } + + @Test + public void testPermissionIdRejectsIntOverflow() { + assertRejected("4294967297"); + assertRejected("99999999999"); + } + + @Test + public void testPermissionIdRejectsNonNumber() { + assertRejected("\"abc\""); + assertRejected("true"); + assertRejected("[1]"); + } + + @Test + public void testPermissionIdRejectsExplicitNull() { + InvalidParameterException e = Assert.assertThrows(InvalidParameterException.class, + () -> applyPermissionId("null")); + Assert.assertTrue(e.getMessage().contains(Util.PERMISSION_ID)); + } + + @Test + public void testAbsentPermissionIdLeavesTransactionUnchanged() { + JSONObject jsonObject = JSONObject.parseObject("{\"amount\":1}"); + Transaction transaction = buildTransferTransaction(); + Assert.assertEquals(0, + permissionIdOf(Util.setTransactionPermissionId(jsonObject, transaction))); + } + + @Test + public void testPermissionIdNotPositiveLeavesTransactionUnchanged() { + Assert.assertEquals(0, permissionIdOf(applyPermissionId("0"))); + Assert.assertEquals(0, permissionIdOf(applyPermissionId("-1"))); + } } From 7bca8a96ff38cb58713d7a04e3be8bd02f8b5be4 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 25 Aug 2026 19:07:39 +0800 Subject: [PATCH 03/14] fix(api): answer an unusable address identically on every endpoint getAddress reports every way a request can fail to name an address as one fixed message, and the reward and brokerage servlets write it, so the fullnode, solidity and PBFT endpoints no longer differ on a given malformed request. An absent address is one of those ways: it used to be answered with the service default. --- .../services/http/GetBrokerageServlet.java | 21 +- .../core/services/http/GetRewardServlet.java | 19 +- .../org/tron/core/services/http/Util.java | 46 ++++- .../http/AddressQueryServletTestBase.java | 183 ++++++++++++++++++ .../http/GetBrokerageServletTest.java | 96 +++------ .../services/http/GetRewardServletTest.java | 140 +++----------- 6 files changed, 298 insertions(+), 207 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java diff --git a/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java b/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java index b735878d1e1..e7f1aa88cbb 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java @@ -3,7 +3,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; -import org.bouncycastle.util.encoders.DecoderException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.tron.core.db.Manager; @@ -17,18 +16,22 @@ public class GetBrokerageServlet extends RateLimiterServlet { private Manager manager; protected void doGet(HttpServletRequest request, HttpServletResponse response) { + byte[] address; + try { + address = Util.getAddress(request); + } catch (IllegalArgumentException e) { + Util.writeAuditedError(e.getMessage(), response); + return; + } catch (Exception e) { + Util.processError(e, response); + return; + } try { - int value = 0; - byte[] address = Util.getAddress(request); long cycle = manager.getDynamicPropertiesStore().getCurrentCycleNumber(); - if (address != null) { - value = manager.getDelegationStore().getBrokerage(cycle, address); - } + int value = manager.getDelegationStore().getBrokerage(cycle, address); response.getWriter().println("{\"brokerage\": " + value + "}"); - } catch (DecoderException | IllegalArgumentException e) { - Util.writeAuditedError(Util.INVALID_ADDRESS_MSG, response); } catch (Exception e) { - Util.processError(e, response); + Util.processServerError(e, response); } } diff --git a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java index 780bab6ac94..d0c20153586 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java @@ -3,7 +3,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; -import org.bouncycastle.util.encoders.DecoderException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.tron.core.db.Manager; @@ -17,18 +16,22 @@ public class GetRewardServlet extends RateLimiterServlet { private Manager manager; protected void doGet(HttpServletRequest request, HttpServletResponse response) { + byte[] address; try { - long value = 0; - byte[] address = Util.getAddress(request); - if (address != null) { - value = manager.getMortgageService().queryReward(address); - } + address = Util.getAddress(request); + } catch (IllegalArgumentException e) { + Util.writeAuditedError(e.getMessage(), response); + return; + } catch (Exception e) { + Util.processError(e, response); + return; + } + try { + long value = manager.getMortgageService().queryReward(address); String out = JsonFormat.isInt64AsString() ? "{\"reward\": \"" + value + "\"}" : "{\"reward\": " + value + "}"; response.getWriter().println(out); - } catch (DecoderException | IllegalArgumentException e) { - Util.writeAuditedError(Util.INVALID_ADDRESS_MSG, response); } catch (Exception e) { Util.processServerError(e, response); } diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index 033b40e2761..2c0dd1cd155 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -25,6 +25,7 @@ import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; +import org.bouncycastle.util.encoders.DecoderException; import org.bouncycastle.util.encoders.Hex; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.MimeTypes; @@ -40,6 +41,7 @@ import org.tron.common.crypto.Hash; import org.tron.common.parameter.CommonParameter; import org.tron.common.utils.ByteArray; +import org.tron.common.utils.DecodeUtil; import org.tron.common.utils.Sha256Hash; import org.tron.core.Constant; import org.tron.core.actuator.TransactionFactory; @@ -77,6 +79,7 @@ public class Util { private static final String INVALID_PERMISSION_ID = "invalid " + PERMISSION_ID + ": expect a 32-bit integer"; private static final int MAX_JSON_INTEGER_VALUE_LENGTH = 64; + private static final String INVALID_JSON_BODY = "INVALID JSON body"; public static final String VISIBLE = "visible"; public static final String INT64_AS_STRING_PARAM = "int64_as_string"; public static final String TRANSACTION = "transaction"; @@ -623,16 +626,43 @@ public static void printAccount(Account reply, HttpServletResponse response, Boo } } + /** + * Returns the address the request carries. Every way the request can fail to name one is + * reported as an IllegalArgumentException whose message is the fixed text the caller is + * answered with, so that the address-keyed endpoints and their solidity/PBFT mirrors answer a + * given malformed request identically without each having to classify the failure. Nothing + * derived from the request may go into that message: it is written straight to the response. + */ public static byte[] getAddress(HttpServletRequest request) throws Exception { - byte[] address = null; String addressParam = "address"; - String addressStr = checkGetParam(request, addressParam); - if (StringUtils.isNotBlank(addressStr)) { - if (StringUtils.startsWith(addressStr, Constant.ADD_PRE_FIX_STRING_MAINNET)) { - address = Hex.decode(addressStr); - } else { - address = decodeFromBase58Check(addressStr); - } + String addressStr; + try { + addressStr = checkGetParam(request, addressParam); + } catch (JSONException e) { + throw new IllegalArgumentException(INVALID_JSON_BODY); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + } + if (StringUtils.isBlank(addressStr)) { + throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + } + + boolean hex = StringUtils.startsWith(addressStr, Constant.ADD_PRE_FIX_STRING_MAINNET); + // bound the hex input before decoding, mirroring the base58 length short-circuit + if (hex && addressStr.length() != DecodeUtil.ADDRESS_SIZE) { + throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + } + + byte[] address; + try { + address = hex ? Hex.decode(addressStr) : decodeFromBase58Check(addressStr); + } catch (DecoderException | IllegalArgumentException exception) { + // both decoders name the offending character and its offset, which is caller input + throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + } + // base58 is validated inside the decoder; hex used to be returned unchecked + if (address == null || (hex && !DecodeUtil.addressValid(address))) { + throw new IllegalArgumentException(INVALID_ADDRESS_MSG); } return address; } diff --git a/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java b/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java new file mode 100644 index 00000000000..9e6e218e995 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java @@ -0,0 +1,183 @@ +package org.tron.core.services.http; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.util.Arrays; +import java.util.Collections; +import org.apache.commons.lang3.StringUtils; +import org.junit.Assert; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.common.BaseTest; +import org.tron.common.utils.StringUtil; +import org.tron.core.Wallet; +import org.tron.json.JSON; +import org.tron.json.JSONObject; + +/** + * The shared address-parameter contract for the reward and brokerage endpoints. The cases live + * here rather than in each servlet's test so that the endpoints cannot drift apart in how they + * answer a given malformed request. + */ +public abstract class AddressQueryServletTestBase extends BaseTest { + + static final String INVALID_ADDRESS_ERROR = "INVALID address"; + + /** Invokes the concrete servlet's GET handler. */ + protected abstract void invokeGet(MockHttpServletRequest request, + MockHttpServletResponse response); + + /** Invokes the concrete servlet's POST handler. */ + protected abstract void invokePost(MockHttpServletRequest request, + MockHttpServletResponse response); + + /** The key this endpoint answers with, for instance {@code reward}. */ + protected abstract String valueKey(); + + /** What this endpoint answers for a well-formed address it holds no state for. */ + protected abstract int valueWithoutState(); + + protected static MockHttpServletRequest postRequest(String contentType) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setContentType(contentType); + request.setCharacterEncoding(UTF_8.name()); + return request; + } + + protected static MockHttpServletRequest getRequest(String address) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + protected static MockHttpServletRequest formRequest(String address) { + MockHttpServletRequest request = postRequest("application/x-www-form-urlencoded"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + protected static MockHttpServletRequest jsonRequest(String address) { + return jsonRequest(address, "application/json"); + } + + protected static MockHttpServletRequest jsonRequest(String address, String contentType) { + MockHttpServletRequest request = postRequest(contentType); + String json = address == null ? "{}" + : JSON.toJSONString(Collections.singletonMap("address", address)); + request.setContent(json.getBytes(UTF_8)); + return request; + } + + /** A base58check payload whose first byte is not the address prefix. */ + protected static String wrongPrefixAddress() { + byte[] address = new byte[21]; + address[0] = (byte) (Wallet.getAddressPreFixByte() + 1); + return StringUtil.encode58Check(address); + } + + /** A well-formed address that no store holds anything for. */ + protected static String canonicalNoStateAddress() { + byte[] address = new byte[21]; + Arrays.fill(address, (byte) 7); + address[0] = Wallet.getAddressPreFixByte(); + return StringUtil.encode58Check(address); + } + + /** Runs the request through the servlet with the verb it carries, and returns the raw body. */ + protected String invoke(MockHttpServletRequest request) throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + if ("GET".equals(request.getMethod())) { + invokeGet(request, response); + } else { + invokePost(request, response); + } + return response.getContentAsString(); + } + + private void assertInvalid(MockHttpServletRequest request, String submitted) throws Exception { + String body = invoke(request); + JSONObject result = JSONObject.parseObject(body); + Assert.assertEquals(INVALID_ADDRESS_ERROR, result.get("Error")); + Assert.assertNull(result.get(valueKey())); + if (submitted != null && !submitted.isEmpty()) { + Assert.assertFalse("the error must not echo the submitted address", body.contains(submitted)); + } + } + + private void assertValue(MockHttpServletRequest request, int expected) throws Exception { + JSONObject result = JSONObject.parseObject(invoke(request)); + Assert.assertEquals(expected, (int) result.get(valueKey())); + Assert.assertNull(result.get("Error")); + } + + @Test + public void getAndPostRejectMalformedAddresses() throws Exception { + String invalidBase58 = "Taaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + // '0' is outside the base58 alphabet, so the decoder itself raises + String illegalBase58Char = "Taaaaaaaaaaaaaaaaa0aaaaaaaaaaaaaaa"; + String invalidHex = "41zz00000000000000000000000000000000000000"; + // unlike a raw "42..." string this reaches the prefix check instead of failing the alphabet + String wrongPrefix = wrongPrefixAddress(); + String oversizedHex = "41" + StringUtils.repeat('0', 8192); + String oversizedBase58 = "T" + StringUtils.repeat('a', 8192); + + assertInvalid(getRequest(invalidBase58), invalidBase58); + assertInvalid(getRequest(illegalBase58Char), illegalBase58Char); + assertInvalid(getRequest(wrongPrefix), wrongPrefix); + assertInvalid(getRequest(oversizedHex), oversizedHex); + assertInvalid(getRequest(oversizedBase58), oversizedBase58); + assertInvalid(jsonRequest(oversizedBase58), oversizedBase58); + assertInvalid(jsonRequest(invalidBase58), invalidBase58); + assertInvalid(jsonRequest(invalidHex), invalidHex); + } + + /** + * An address that is absent or blank is rejected like any other address the endpoint cannot + * use, on every verb and body encoding. It used to answer with the service default instead. + */ + @Test + public void missingOrBlankAddressIsRejected() throws Exception { + assertInvalid(getRequest(null), null); + assertInvalid(getRequest(""), null); + assertInvalid(formRequest(null), null); + assertInvalid(formRequest(""), null); + assertInvalid(jsonRequest(null), null); + } + + @Test + public void validAddressWithoutStateRetainsServiceDefault() throws Exception { + assertValue(getRequest(canonicalNoStateAddress()), valueWithoutState()); + } + + /** + * The charset-suffixed content type is what browsers actually send, and it must take the + * json-body branch of Util.checkGetParam rather than the form-parameter branch. The body + * carries a well-formed address: on the form branch no address would be found at all and + * the answer would be a rejection instead of the value. + */ + @Test + public void jsonBodyWithCharsetSuffixIsParsed() throws Exception { + assertValue(jsonRequest(canonicalNoStateAddress(), "application/json; charset=utf-8"), + valueWithoutState()); + } + + @Test + public void malformedJsonBodyIsRejectedWithoutEchoingIt() throws Exception { + String marker = "UniqueMarkerValue"; + MockHttpServletRequest request = postRequest("application/json"); + request.setContent(("{\"address\":" + marker + "}").getBytes(UTF_8)); + + String body = invoke(request); + Assert.assertFalse(body.contains(marker)); + JSONObject result = JSONObject.parseObject(body); + Assert.assertEquals("INVALID JSON body", result.get("Error")); + Assert.assertNull(result.get(valueKey())); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/GetBrokerageServletTest.java b/framework/src/test/java/org/tron/core/services/http/GetBrokerageServletTest.java index 9b37c2e4205..29a67d387b0 100644 --- a/framework/src/test/java/org/tron/core/services/http/GetBrokerageServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/GetBrokerageServletTest.java @@ -1,20 +1,18 @@ package org.tron.core.services.http; -import java.io.UnsupportedEncodingException; import javax.annotation.Resource; import org.junit.Assert; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; -import org.tron.common.BaseTest; import org.tron.common.TestConstants; import org.tron.core.config.args.Args; import org.tron.json.JSONObject; -public class GetBrokerageServletTest extends BaseTest { +public class GetBrokerageServletTest extends AddressQueryServletTestBase { @Resource - private GetBrokerageServlet getBrokerageServlet; + private GetBrokerageServlet getBrokerageServlet; static { Args.setParam( @@ -24,84 +22,40 @@ public class GetBrokerageServletTest extends BaseTest { ); } - public MockHttpServletRequest createRequest(String contentType) { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setMethod("POST"); - request.setContentType(contentType); - request.setCharacterEncoding("UTF-8"); - return request; + @Override + protected void invokeGet(MockHttpServletRequest request, MockHttpServletResponse response) { + getBrokerageServlet.doGet(request, response); } - @Test - public void getBrokerageValueByJsonTest() { - int expect = 20; - String jsonParam = "{\"address\": \"TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W\"}"; - MockHttpServletRequest request = createRequest("application/json"); - request.setContent(jsonParam.getBytes()); - MockHttpServletResponse response = new MockHttpServletResponse(); + @Override + protected void invokePost(MockHttpServletRequest request, MockHttpServletResponse response) { getBrokerageServlet.doPost(request, response); - try { - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int brokerage = (int)result.get("brokerage"); - Assert.assertEquals(expect, brokerage); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } } + @Override + protected String valueKey() { + return "brokerage"; + } - @Test - public void getBrokerageByJsonUTF8Test() { - int expect = 20; - String jsonParam = "{\"address\": \"TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W\"}"; - MockHttpServletRequest request = createRequest("application/json; charset=utf-8"); - request.setContent(jsonParam.getBytes()); - MockHttpServletResponse response = new MockHttpServletResponse(); - getBrokerageServlet.doPost(request, response); - try { - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int brokerage = (int)result.get("brokerage"); - Assert.assertEquals(expect, brokerage); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } + /** An address the delegation store holds nothing for still answers the default brokerage. */ + @Override + protected int valueWithoutState() { + return 20; } @Test - public void getBrokerageValueTest() { - int expect = 20; - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - request.addParameter("address", "TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W"); - MockHttpServletResponse response = new MockHttpServletResponse(); - getBrokerageServlet.doPost(request, response); - try { - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int brokerage = (int)result.get("brokerage"); - Assert.assertEquals(expect, brokerage); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } + public void getBrokerageValueByJsonTest() throws Exception { + MockHttpServletRequest request = jsonRequest("TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W"); + + JSONObject result = JSONObject.parseObject(invoke(request)); + Assert.assertEquals(20, (int) result.get("brokerage")); } @Test - public void getByBlankParamTest() { - int expect = 0; - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - request.addParameter("address", ""); - MockHttpServletResponse response = new MockHttpServletResponse(); - getBrokerageServlet.doPost(request, response); - try { - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int brokerage = (int)result.get("brokerage"); - Assert.assertEquals(expect, brokerage); - String content = (String) result.get("Error"); - Assert.assertNull(content); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } + public void getBrokerageValueTest() throws Exception { + MockHttpServletRequest request = formRequest("TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W"); + + JSONObject result = JSONObject.parseObject(invoke(request)); + Assert.assertEquals(20, (int) result.get("brokerage")); } } diff --git a/framework/src/test/java/org/tron/core/services/http/GetRewardServletTest.java b/framework/src/test/java/org/tron/core/services/http/GetRewardServletTest.java index 9afa5607a66..438e4545706 100644 --- a/framework/src/test/java/org/tron/core/services/http/GetRewardServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/GetRewardServletTest.java @@ -2,7 +2,6 @@ import static org.tron.common.utils.Commons.decodeFromBase58Check; -import java.io.UnsupportedEncodingException; import javax.annotation.Resource; import lombok.extern.slf4j.Slf4j; import org.junit.Assert; @@ -10,7 +9,6 @@ import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; -import org.tron.common.BaseTest; import org.tron.common.TestConstants; import org.tron.core.config.args.Args; import org.tron.core.db.Manager; @@ -19,7 +17,7 @@ import org.tron.json.JSONObject; @Slf4j -public class GetRewardServletTest extends BaseTest { +public class GetRewardServletTest extends AddressQueryServletTestBase { @Resource private Manager manager; @@ -31,22 +29,30 @@ public class GetRewardServletTest extends BaseTest { private DelegationStore delegationStore; @Resource - GetRewardServlet getRewardServlet; + private GetRewardServlet getRewardServlet; static { - Args.setParam( - new String[]{ - "--output-directory", dbPath(), - }, TestConstants.TEST_CONF - ); + Args.setParam(new String[]{"--output-directory", dbPath()}, TestConstants.TEST_CONF); } - public MockHttpServletRequest createRequest(String contentType) { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setMethod("POST"); - request.setContentType(contentType); - request.setCharacterEncoding("UTF-8"); - return request; + @Override + protected void invokeGet(MockHttpServletRequest request, MockHttpServletResponse response) { + getRewardServlet.doGet(request, response); + } + + @Override + protected void invokePost(MockHttpServletRequest request, MockHttpServletResponse response) { + getRewardServlet.doPost(request, response); + } + + @Override + protected String valueKey() { + return "reward"; + } + + @Override + protected int valueWithoutState() { + return 0; } @Before @@ -58,107 +64,19 @@ public void init() { } @Test - public void getRewardValueByJsonTest() { - int expect = 138181; - String jsonParam = "{\"address\": \"TNboetpFgv9SqMoHvaVt626NLXETnbdW1K\"}"; - MockHttpServletRequest request = createRequest("application/json"); - MockHttpServletResponse response = new MockHttpServletResponse(); - request.setContent(jsonParam.getBytes()); - try { - getRewardServlet.doPost(request, response); - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int reward = (int)result.get("reward"); - Assert.assertEquals(expect, reward); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } - } + public void getRewardValueByJsonTest() throws Exception { + MockHttpServletRequest request = jsonRequest("TNboetpFgv9SqMoHvaVt626NLXETnbdW1K"); - @Test - public void getRewardByJsonUTF8Test() { - int expect = 138181; - String jsonParam = "{\"address\": \"TNboetpFgv9SqMoHvaVt626NLXETnbdW1K\"}"; - MockHttpServletRequest request = createRequest("application/json; charset=utf-8"); - MockHttpServletResponse response = new MockHttpServletResponse(); - request.setContent(jsonParam.getBytes()); - try { - getRewardServlet.doPost(request, response); - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int reward = (int)result.get("reward"); - Assert.assertEquals(expect, reward); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } + JSONObject result = JSONObject.parseObject(invoke(request)); + Assert.assertEquals(138181, (int) result.get("reward")); } @Test - public void getRewardValueTest() { - int expect = 138181; - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - MockHttpServletResponse response = new MockHttpServletResponse(); + public void getRewardValueTest() throws Exception { mortgageService.payStandbyWitness(); - request.addParameter("address", "TNboetpFgv9SqMoHvaVt626NLXETnbdW1K"); - getRewardServlet.doPost(request, response); - try { - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int reward = (int)result.get("reward"); - Assert.assertEquals(expect, reward); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } - } - - @Test - public void getByBlankParamTest() { - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - MockHttpServletResponse response = new MockHttpServletResponse(); - request.addParameter("address", ""); - GetRewardServlet getRewardServlet = new GetRewardServlet(); - getRewardServlet.doPost(request, response); - try { - String contentAsString = response.getContentAsString(); - JSONObject result = JSONObject.parseObject(contentAsString); - int reward = (int)result.get("reward"); - Assert.assertEquals(0, reward); - String content = (String) result.get("Error"); - Assert.assertNull(content); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } - } + MockHttpServletRequest request = formRequest("TNboetpFgv9SqMoHvaVt626NLXETnbdW1K"); - @Test - public void getRewardByOversizedValidCharAddressTest() { - // 41-char, all-valid-Base58 address: the length guard returns null -> reward 0. - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - MockHttpServletResponse response = new MockHttpServletResponse(); - request.addParameter("address", "T" + new String(new char[40]).replace('\0', 'a')); - new GetRewardServlet().doPost(request, response); - try { - JSONObject result = JSONObject.parseObject(response.getContentAsString()); - Assert.assertEquals(0, (int) result.get("reward")); - Assert.assertNull(result.get("Error")); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } + JSONObject result = JSONObject.parseObject(invoke(request)); + Assert.assertEquals(138181, (int) result.get("reward")); } - - @Test - public void getRewardByOversizedIllegalCharAddressTest() { - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - MockHttpServletResponse response = new MockHttpServletResponse(); - request.addParameter("address", "T" + new String(new char[40]).replace('\0', '0')); - new GetRewardServlet().doPost(request, response); - try { - JSONObject result = JSONObject.parseObject(response.getContentAsString()); - Assert.assertEquals(0, (int) result.get("reward")); - Assert.assertNull(result.get("Error")); - } catch (UnsupportedEncodingException e) { - Assert.fail(e.getMessage()); - } - } - } From 045f929ec1eb166eaf9bd324e3d0f14dfdbf898b Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 25 Aug 2026 19:07:39 +0800 Subject: [PATCH 04/14] fix(shielded): bound the shielded TRC-20 description counts Mint takes exactly one receive description and transfer takes one or two of each. The counts are checked before the merge loops, because ByteUtil.merge copies the accumulated buffer on every iteration and checking afterwards would make an unbounded list quadratic. --- .../zen/ShieldedTRC20ParametersBuilder.java | 21 ++-- .../java/org/tron/core/WalletMockTest.java | 3 + .../ShieldedTRC20ParametersBuilderTest.java | 119 ++++++++++++++++++ 3 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java diff --git a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java index 4bef932971d..1a12068e924 100644 --- a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java +++ b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java @@ -402,6 +402,9 @@ private String mintParamsToHexString(GrpcAPI.ShieldedTRC20Parameters mintParams, if (value.compareTo(BigInteger.ZERO) <= 0) { throw new IllegalArgumentException("require the value be positive"); } + if (mintParams.getReceiveDescriptionCount() != 1) { + throw new IllegalArgumentException("invalid mint description number"); + } ShieldContract.ReceiveDescription revDesc = mintParams.getReceiveDescription(0); byte[] zeros = new byte[12]; @@ -422,12 +425,22 @@ private String mintParamsToHexString(GrpcAPI.ShieldedTRC20Parameters mintParams, private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transferParams, List spendAuthoritySignature, boolean withAsk) { + List spendDescs = transferParams.getSpendDescriptionList(); + List recvDescs = transferParams.getReceiveDescriptionList(); + long spendCount = spendDescs.size(); + long recvCount = recvDescs.size(); + if (spendCount < 1 || spendCount > 2) { + throw new IllegalArgumentException("invalid transfer input number"); + } + if (recvCount < 1 || recvCount > 2) { + throw new IllegalArgumentException("invalid transfer output number"); + } + byte[] input = new byte[0]; byte[] spendAuthSig = new byte[0]; byte[] output = new byte[0]; byte[] c = new byte[0]; byte[] bindingSig; - List spendDescs = transferParams.getSpendDescriptionList(); for (ShieldContract.SpendDescription spendDesc : spendDescs) { input = ByteUtil.merge(input, spendDesc.getNullifier().toByteArray(), @@ -441,10 +454,6 @@ private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transfe spendAuthSig, spendDesc.getSpendAuthoritySignature().toByteArray()); } } - long spendCount = spendDescs.size(); - if (spendCount < 1 || spendCount > 2) { - throw new IllegalArgumentException("invalid transfer input number"); - } if (!withAsk) { if (spendCount == 1) { spendAuthSig = spendAuthoritySignature.get(0).getValue().toByteArray(); @@ -458,7 +467,6 @@ private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transfe byte[] spendCountBytes = ByteUtil.longTo32Bytes(spendCount); byte[] authOffsetBytes = ByteUtil.longTo32Bytes(192 + 32 + 320 * spendCount); - List recvDescs = transferParams.getReceiveDescriptionList(); for (ShieldContract.ReceiveDescription recvDesc : recvDescs) { output = ByteUtil.merge(output, recvDesc.getNoteCommitment().toByteArray(), @@ -474,7 +482,6 @@ private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transfe ); } - long recvCount = recvDescs.size(); byte[] recvCountBytes = ByteUtil.longTo32Bytes(recvCount); byte[] outputOffsetbytes = ByteUtil .longTo32Bytes(192 + 32 + 320 * spendCount + 32 + 64 * spendCount); diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index 7874af854a6..fee73202b7e 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -907,6 +907,9 @@ public void testGetTriggerInputForShieldedTRC20Contract1() GrpcAPI.ShieldedTRC20Parameters shieldedTRC20Parameters = GrpcAPI.ShieldedTRC20Parameters.newBuilder() .addSpendDescription(spendDescription) + // a transfer requires exactly one receive description to clear the output-arity gate; + // the rejection of a wrong count is covered by WalletShieldedDescriptionValidationTest + .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) .setParameterType("transfer") .build(); GrpcAPI.BytesMessage bytesMessage = diff --git a/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java b/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java new file mode 100644 index 00000000000..6681b4c6946 --- /dev/null +++ b/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java @@ -0,0 +1,119 @@ +package org.tron.core.zen; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.mockStatic; + +import java.math.BigInteger; +import java.util.Collections; +import java.util.List; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI.BytesMessage; +import org.tron.api.GrpcAPI.ShieldedTRC20Parameters; +import org.tron.common.utils.ByteUtil; +import org.tron.protos.contract.ShieldContract; + +public class ShieldedTRC20ParametersBuilderTest { + + private static final String GOLDEN_ONE_TO_ONE = + "00000000000000000000000000000000000000000000000000000000000000c0" + + "0000000000000000000000000000000000000000000000000000000000000220" + + "0000000000000000000000000000000000000000000000000000000000000280" + + "00000000000000000000000000000000000000000000000000000000000003c0" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "000000000000000000000000"; + + private static final String GOLDEN_TWO_TO_TWO = + "00000000000000000000000000000000000000000000000000000000000000c0" + + "0000000000000000000000000000000000000000000000000000000000000360" + + "0000000000000000000000000000000000000000000000000000000000000400" + + "0000000000000000000000000000000000000000000000000000000000000660" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "000000000000000000000000000000000000000000000000"; + + private static final String GOLDEN_ONE_TO_TWO = + "00000000000000000000000000000000000000000000000000000000000000c0" + + "0000000000000000000000000000000000000000000000000000000000000220" + + "0000000000000000000000000000000000000000000000000000000000000280" + + "00000000000000000000000000000000000000000000000000000000000004e0" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "000000000000000000000000000000000000000000000000"; + + private static final String GOLDEN_TWO_TO_ONE = + "00000000000000000000000000000000000000000000000000000000000000c0" + + "0000000000000000000000000000000000000000000000000000000000000360" + + "0000000000000000000000000000000000000000000000000000000000000400" + + "0000000000000000000000000000000000000000000000000000000000000540" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000002" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "0000000000000000000000000000000000000000000000000000000000000001" + + "000000000000000000000000"; + + private static ShieldedTRC20ParametersBuilder transferBuilder() throws Exception { + return new ShieldedTRC20ParametersBuilder("transfer"); + } + + private static ShieldedTRC20Parameters params(int spends, int receives) { + ShieldedTRC20Parameters.Builder parameters = ShieldedTRC20Parameters.newBuilder(); + for (int i = 0; i < spends; i++) { + parameters.addSpendDescription(ShieldContract.SpendDescription.getDefaultInstance()); + } + for (int i = 0; i < receives; i++) { + parameters.addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()); + } + return parameters.build(); + } + + private static String run(int spends, int receives) throws Exception { + return run(spends, receives, Collections.emptyList(), true); + } + + private static String run(int spends, int receives, List signatures, + boolean withAsk) throws Exception { + return transferBuilder().getTriggerContractInput( + params(spends, receives), signatures, BigInteger.ZERO, withAsk, new byte[21]); + } + + @Test + public void invalidCountsAreRejectedBeforeAnyMerge() throws Exception { + Object[][] invalidCounts = { + {0, 1, "invalid transfer input number"}, + {3, 1, "invalid transfer input number"}, + {1, 0, "invalid transfer output number"}, + {1, 3, "invalid transfer output number"}, + {1, 10_000, "invalid transfer output number"}, + }; + try (MockedStatic byteUtil = mockStatic(ByteUtil.class)) { + for (Object[] counts : invalidCounts) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> run((int) counts[0], (int) counts[1])); + assertEquals(counts[2], exception.getMessage()); + } + byteUtil.verifyNoInteractions(); + } + } + + /** + * The asymmetric shapes are the ones that pin the arithmetic: both offsets mix the two counts, + * so with spendCount == recvCount a transposition of the two produces identical bytes and the + * symmetric vectors alone would pass under it. + */ + @Test + public void validOneAndTwoEntryOutputsRemainByteForByteCompatible() throws Exception { + assertEquals(GOLDEN_ONE_TO_ONE, run(1, 1)); + assertEquals(GOLDEN_TWO_TO_TWO, run(2, 2)); + assertEquals(GOLDEN_ONE_TO_TWO, run(1, 2)); + assertEquals(GOLDEN_TWO_TO_ONE, run(2, 1)); + } +} From 8e907278d8d0aa2599f5a23b81dd972aebd23c86 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 25 Aug 2026 19:07:39 +0800 Subject: [PATCH 05/14] fix(shielded): report unusable shielded trigger input as a validation failure An amount string past 80 characters and a trigger input the builder rejects are answered as a ContractValidateException naming the argument, instead of escaping the wallet as an IllegalArgumentException. --- .../src/main/java/org/tron/core/Wallet.java | 26 ++- .../WalletShieldedAmountValidationTest.java | 148 ++++++++++++++++++ ...lletShieldedDescriptionValidationTest.java | 76 +++++++++ 3 files changed, 246 insertions(+), 4 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/WalletShieldedAmountValidationTest.java create mode 100644 framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 936ad12337d..afe3a9d33f9 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -261,6 +261,9 @@ public class Wallet { "Shielded transaction API is disabled; " + "set node.allowShieldedTransactionApi=true to enable."; private static final String PAYMENT_ADDRESS_FORMAT_WRONG = "paymentAddress format is wrong"; + // the authoritative bound is checkBigIntegerRange: uint256 max is 78 decimal digits, + // this only keeps the string short enough to convert cheaply + private static final int MAX_SHIELDED_AMOUNT_LENGTH = 80; private static final String SHIELDED_TRANSACTION_SCAN_RANGE = "request requires start_block_index >= 0 && end_block_index > " + "start_block_index && end_block_index - start_block_index <= 1000"; @@ -4222,6 +4225,9 @@ private BigInteger getBigIntegerFromString(String in) { if (trimmedIn.length() == 0) { return BigInteger.ZERO; } + if (trimmedIn.length() > MAX_SHIELDED_AMOUNT_LENGTH) { + throw new IllegalArgumentException("invalid shielded amount"); + } return new BigInteger(trimmedIn, 10); } @@ -4333,7 +4339,12 @@ public BytesMessage getTriggerInputForShieldedTRC20Contract( ShieldedTRC20Parameters shieldedTRC20Parameters = request.getShieldedTRC20Parameters(); List spendAuthoritySignature = request.getSpendAuthoritySignatureList(); - BigInteger value = getBigIntegerFromString(request.getAmount()); + BigInteger value; + try { + value = getBigIntegerFromString(request.getAmount()); + } catch (IllegalArgumentException e) { + throw new ContractValidateException("invalid amount"); + } checkBigIntegerRange(value); byte[] transparentToAddress = request.getTransparentToAddress().toByteArray(); byte[] transparentToAddressTvm = new byte[20]; @@ -4398,9 +4409,16 @@ public BytesMessage getTriggerInputForShieldedTRC20Contract( } parametersBuilder.setBurnCiphertext(burnCiper); } - String input = parametersBuilder - .getTriggerContractInput(shieldedTRC20Parameters, spendAuthoritySignature, value, false, - transparentToAddressTvm); + String input; + try { + input = parametersBuilder + .getTriggerContractInput(shieldedTRC20Parameters, spendAuthoritySignature, value, false, + transparentToAddressTvm); + } catch (IllegalArgumentException e) { + // the builder names the offending argument in fixed strings; none of them echoes an input + throw new ContractValidateException( + "invalid shielded TRC-20 trigger input: " + e.getMessage(), e); + } if (Objects.isNull(input)) { throw new ZksnarkException("generate the trigger contract parameters error"); } diff --git a/framework/src/test/java/org/tron/core/WalletShieldedAmountValidationTest.java b/framework/src/test/java/org/tron/core/WalletShieldedAmountValidationTest.java new file mode 100644 index 00000000000..58c65a71449 --- /dev/null +++ b/framework/src/test/java/org/tron/core/WalletShieldedAmountValidationTest.java @@ -0,0 +1,148 @@ +package org.tron.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.math.BigInteger; +import org.apache.commons.lang3.StringUtils; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI; +import org.tron.common.parameter.CommonParameter; +import org.tron.core.config.args.Args; +import org.tron.core.exception.ContractValidateException; +import org.tron.core.exception.ZksnarkException; + +public class WalletShieldedAmountValidationTest { + + // both methods under test are pure and never touch instance state, so one wallet is enough; + // constructing one generates a keypair, which is not worth paying for on every invocation + private static Wallet wallet; + private static Method bigIntegerFromString; + private static Method checkBigIntegerRange; + + @BeforeClass + public static void setUpClass() throws Exception { + wallet = new Wallet(); + bigIntegerFromString = + Wallet.class.getDeclaredMethod("getBigIntegerFromString", String.class); + bigIntegerFromString.setAccessible(true); + checkBigIntegerRange = + Wallet.class.getDeclaredMethod("checkBigIntegerRange", BigInteger.class); + checkBigIntegerRange.setAccessible(true); + } + + private static BigInteger parse(String value) throws Exception { + try { + return (BigInteger) bigIntegerFromString.invoke(wallet, value); + } catch (InvocationTargetException exception) { + throw (Exception) exception.getCause(); + } + } + + private static void checkRange(BigInteger value) throws Exception { + try { + checkBigIntegerRange.invoke(wallet, value); + } catch (InvocationTargetException exception) { + throw (Exception) exception.getCause(); + } + } + + @Test + public void amountLengthBoundaryIsAppliedAfterTrimAndBeforeParsing() throws Exception { + assertEquals(BigInteger.ZERO, parse(" ")); + assertEquals(BigInteger.ZERO, parse(" " + StringUtils.repeat('0', 80) + " ")); + + String oversized = StringUtils.repeat('9', 81); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> parse(oversized)); + assertEquals("invalid shielded amount", exception.getMessage()); + assertFalse(exception.getMessage().contains(oversized)); + assertThrows(IllegalArgumentException.class, () -> parse(StringUtils.repeat('0', 81))); + } + + @Test + public void uint256AndSignRangeChecksRemainAuthoritative() throws Exception { + BigInteger maximum = BigInteger.ONE.shiftLeft(256).subtract(BigInteger.ONE); + assertEquals(maximum, parse(maximum.toString())); + checkRange(maximum); + + ContractValidateException tooLarge = assertThrows(ContractValidateException.class, + () -> checkRange(parse(BigInteger.ONE.shiftLeft(256).toString()))); + assertTrue(tooLarge.getMessage().contains("256 bits")); + + ContractValidateException negative = assertThrows(ContractValidateException.class, + () -> checkRange(parse("-1"))); + assertTrue(negative.getMessage().contains("non-negative")); + assertEquals(BigInteger.valueOf(123), parse(" 123 ")); + } + + /** Runs the public entrypoint with the shielded api enabled, so the amount is reached. */ + private static void triggerInputWithAmount(String amount) throws Exception { + GrpcAPI.ShieldedTRC20TriggerContractParameters request = + GrpcAPI.ShieldedTRC20TriggerContractParameters.newBuilder() + .setAmount(amount) + .setShieldedTRC20Parameters(GrpcAPI.ShieldedTRC20Parameters.newBuilder() + .setParameterType("transfer") + .build()) + .build(); + CommonParameter commonParameter = mock(Args.class); + try (MockedStatic mocked = mockStatic(CommonParameter.class)) { + when(CommonParameter.getInstance()).thenReturn(commonParameter); + when(commonParameter.isAllowShieldedTransactionApi()).thenReturn(true); + wallet.getTriggerInputForShieldedTRC20Contract(request); + } + } + + /** + * getBigIntegerFromString reports an overlong amount, and a malformed one, with unchecked + * exceptions. The entrypoint declares only ZksnarkException and ContractValidateException, and + * the http error sanitizer answers an untranslated failure with "internal server error" instead + * of the validation error every other amount entrypoint answers with. + */ + @Test + public void unusableAmountsAreReportedAsValidationFailures() { + String[] unusable = {StringUtils.repeat('9', 81), "not-a-number", "1.5"}; + for (String amount : unusable) { + ContractValidateException exception = assertThrows(ContractValidateException.class, + () -> triggerInputWithAmount(amount)); + assertEquals("invalid amount", exception.getMessage()); + } + } + + /** A well-formed amount past the uint256 range still reports the range failure, not "invalid". */ + @Test + public void outOfRangeAmountRetainsRangeValidationMessage() { + ContractValidateException exception = assertThrows(ContractValidateException.class, + () -> triggerInputWithAmount(BigInteger.ONE.shiftLeft(256).toString())); + assertTrue(exception.getMessage().contains("256 bits")); + } + + @Test + public void disabledFeatureGateRunsBeforeAmountOrDescriptionValidation() { + GrpcAPI.ShieldedTRC20TriggerContractParameters request = + GrpcAPI.ShieldedTRC20TriggerContractParameters.newBuilder() + .setAmount(StringUtils.repeat('9', 81)) + .setShieldedTRC20Parameters(GrpcAPI.ShieldedTRC20Parameters.newBuilder() + .setParameterType("transfer") + .build()) + .build(); + CommonParameter commonParameter = mock(Args.class); + try (MockedStatic mocked = mockStatic(CommonParameter.class)) { + when(CommonParameter.getInstance()).thenReturn(commonParameter); + when(commonParameter.isAllowShieldedTransactionApi()).thenReturn(false); + + ZksnarkException exception = assertThrows(ZksnarkException.class, + () -> wallet.getTriggerInputForShieldedTRC20Contract(request)); + assertTrue(exception.getMessage().contains("Shielded transaction API is disabled")); + } + } +} diff --git a/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java b/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java new file mode 100644 index 00000000000..2339459c315 --- /dev/null +++ b/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java @@ -0,0 +1,76 @@ +package org.tron.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI; +import org.tron.api.GrpcAPI.BytesMessage; +import org.tron.common.parameter.CommonParameter; +import org.tron.core.config.args.Args; +import org.tron.core.exception.ContractValidateException; +import org.tron.protos.contract.ShieldContract; + +public class WalletShieldedDescriptionValidationTest { + + private static Wallet wallet; + + @BeforeClass + public static void setUpClass() { + wallet = new Wallet(); + } + + private static void triggerInput(GrpcAPI.ShieldedTRC20Parameters parameters, + String amount, int signatureCount) throws Exception { + GrpcAPI.ShieldedTRC20TriggerContractParameters.Builder request = + GrpcAPI.ShieldedTRC20TriggerContractParameters.newBuilder() + .setAmount(amount) + .setShieldedTRC20Parameters(parameters); + for (int i = 0; i < signatureCount; i++) { + request.addSpendAuthoritySignature(BytesMessage.getDefaultInstance()); + } + + CommonParameter commonParameter = mock(Args.class); + try (MockedStatic mocked = mockStatic(CommonParameter.class)) { + when(CommonParameter.getInstance()).thenReturn(commonParameter); + when(commonParameter.isAllowShieldedTransactionApi()).thenReturn(true); + wallet.getTriggerInputForShieldedTRC20Contract(request.build()); + } + } + + @Test + public void transferDescriptionCardinalityIsReportedAsValidationFailure() { + GrpcAPI.ShieldedTRC20Parameters.Builder parameters = + GrpcAPI.ShieldedTRC20Parameters.newBuilder() + .setParameterType("transfer") + .addSpendDescription(ShieldContract.SpendDescription.getDefaultInstance()); + for (int i = 0; i < 3; i++) { + parameters.addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()); + } + + ContractValidateException exception = assertThrows(ContractValidateException.class, + () -> triggerInput(parameters.build(), "0", 1)); + assertEquals("invalid shielded TRC-20 trigger input: invalid transfer output number", + exception.getMessage()); + } + + @Test + public void mintDescriptionCardinalityIsReportedAsValidationFailure() { + GrpcAPI.ShieldedTRC20Parameters parameters = + GrpcAPI.ShieldedTRC20Parameters.newBuilder() + .setParameterType("mint") + .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) + .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) + .build(); + + ContractValidateException exception = assertThrows(ContractValidateException.class, + () -> triggerInput(parameters, "1", 0)); + assertEquals("invalid shielded TRC-20 trigger input: invalid mint description number", + exception.getMessage()); + } +} From de5ae9260f501f6841a15129ecd3d9148bfc428e Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 29 Sep 2026 12:36:59 +0800 Subject: [PATCH 06/14] fix(api): bound Base58 rendering of oversized address fields JsonFormat renders address fields over 21 bytes as hex instead of Base58 (visible=true). The hello handshake, PBFT SR list, trigger constant check and getContract logs render addresses via WalletUtil.getAddressString, which Base58Check-encodes only 20- and 21-byte input and renders anything else as hex of at most 21 bytes. --- .../org/tron/common/utils/WalletUtil.java | 16 +- .../org/tron/core/db/TransactionTrace.java | 3 +- .../pbft/message/PbftBaseMessage.java | 6 +- .../src/main/java/org/tron/core/Wallet.java | 5 +- .../net/message/handshake/HelloMessage.java | 4 +- .../tron/core/services/http/JsonFormat.java | 3 + .../org/tron/common/utils/WalletUtilTest.java | 67 ++++++ .../core/db/Base58ValidationErrorTest.java | 53 +++++ .../core/net/Base58MessageLoggingTest.java | 214 ++++++++++++++++++ .../BroadcastHexAddressValidationTest.java | 125 ++++++++++ 10 files changed, 484 insertions(+), 12 deletions(-) create mode 100644 framework/src/test/java/org/tron/common/utils/WalletUtilTest.java create mode 100644 framework/src/test/java/org/tron/core/db/Base58ValidationErrorTest.java create mode 100644 framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java create mode 100644 framework/src/test/java/org/tron/core/services/http/BroadcastHexAddressValidationTest.java diff --git a/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java b/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java index 330ce35b6e9..57ef02d4cd0 100644 --- a/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java +++ b/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java @@ -8,7 +8,9 @@ import java.util.List; import java.util.stream.Collectors; import org.tron.common.crypto.Hash; +import org.tron.common.math.StrictMathWrapper; import org.tron.common.parameter.CommonParameter; +import org.tron.core.Constant; import org.tron.core.capsule.ContractCapsule; import org.tron.core.capsule.TransactionCapsule; import org.tron.core.exception.ContractValidateException; @@ -24,6 +26,8 @@ public class WalletUtil { + private static final String INVALID_ADDRESS = "invalid-address"; + public static boolean checkPermissionOperations(Permission permission, Contract contract) throws PermissionException { ByteString operations = permission.getOperations(); @@ -115,9 +119,19 @@ public static boolean isConstant(SmartContract.ABI abi, byte[] selector) { return false; } + public static String getAddressString(byte[] address) { + if (address.length == Constant.STANDARD_ADDRESS_SIZE + || address.length == Constant.TRON_ADDRESS_SIZE) { + return encode58Check(address); + } + int shown = StrictMathWrapper.min(address.length, Constant.TRON_ADDRESS_SIZE); + return INVALID_ADDRESS + "(" + ByteArray.toHexString(Arrays.copyOf(address, shown)) + + (shown < address.length ? "..." : "") + ",len=" + address.length + ")"; + } + public static List getAddressStringList(Collection collection) { return collection.stream() - .map(bytes -> encode58Check(bytes.toByteArray())) + .map(address -> getAddressString(address.toByteArray())) .collect(Collectors.toList()); } diff --git a/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java b/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java index c3776921244..09cfc82570d 100644 --- a/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java +++ b/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java @@ -18,7 +18,6 @@ import org.tron.common.utils.DecodeUtil; import org.tron.common.utils.ForkController; import org.tron.common.utils.Sha256Hash; -import org.tron.common.utils.StringUtil; import org.tron.common.utils.WalletUtil; import org.tron.core.Constant; import org.tron.core.capsule.AccountCapsule; @@ -141,7 +140,7 @@ public void checkIsConstant() throws ContractValidateException, VMIllegalExcepti .get(triggerContractFromTransaction.getContractAddress().toByteArray()); if (contract == null) { throw new ContractValidateException(String.format("contract: %s is not in contract store", - StringUtil.encode58Check(triggerContractFromTransaction + WalletUtil.getAddressString(triggerContractFromTransaction .getContractAddress().toByteArray()))); } diff --git a/consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java b/consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java index dd8a78a28d6..c1c22d0be17 100644 --- a/consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java +++ b/consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java @@ -3,13 +3,12 @@ import com.google.protobuf.InvalidProtocolBufferException; import java.io.IOException; import java.security.SignatureException; -import java.util.stream.Collectors; import org.bouncycastle.util.encoders.Hex; import org.tron.common.crypto.ECKey; import org.tron.common.overlay.message.Message; import org.tron.common.utils.ByteUtil; import org.tron.common.utils.Sha256Hash; -import org.tron.common.utils.StringUtil; +import org.tron.common.utils.WalletUtil; import org.tron.core.capsule.TransactionCapsule; import org.tron.core.exception.P2pException; import org.tron.protos.Protocol.PBFTMessage; @@ -119,8 +118,7 @@ public String getDataString() { private String decode() { try { SRL srList = SRL.parseFrom(pbftMessage.getRawData().getData().toByteArray()); - return "sr list = " + srList.getSrAddressList().stream().map( - bytes -> StringUtil.encode58Check(bytes.toByteArray())).collect(Collectors.toList()); + return "sr list = " + WalletUtil.getAddressStringList(srList.getSrAddressList()); } catch (InvalidProtocolBufferException e) { } return "decode error"; diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index afe3a9d33f9..ed8f349b6f9 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -118,7 +118,6 @@ import org.tron.common.utils.ByteUtil; import org.tron.common.utils.DecodeUtil; import org.tron.common.utils.Sha256Hash; -import org.tron.common.utils.StringUtil; import org.tron.common.utils.Utils; import org.tron.common.utils.WalletUtil; import org.tron.common.zksnark.IncrementalMerkleTreeContainer; @@ -3213,7 +3212,7 @@ public SmartContract getContract(GrpcAPI.BytesMessage bytesMessage) { if (accountCapsule == null) { logger.warn( "Get contract failed, the account {} does not exist or the account " - + "does not have a code hash!", StringUtil.encode58Check(address)); + + "does not have a code hash!", WalletUtil.getAddressString(address)); return null; } @@ -3245,7 +3244,7 @@ public SmartContractDataWrapper getContractInfo(GrpcAPI.BytesMessage bytesMessag if (accountCapsule == null) { logger.warn( "Get contract failed, the account {} does not exist or the account does not have a code " - + "hash!", StringUtil.encode58Check(address)); + + "hash!", WalletUtil.getAddressString(address)); return null; } diff --git a/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java b/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java index 68123c93db6..e66b74bd3b0 100755 --- a/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java @@ -6,7 +6,7 @@ import org.tron.common.utils.ByteArray; import org.tron.common.utils.DecodeUtil; import org.tron.common.utils.Sha256Hash; -import org.tron.common.utils.StringUtil; +import org.tron.common.utils.WalletUtil; import org.tron.core.ChainBaseManager; import org.tron.core.capsule.BlockCapsule; import org.tron.core.config.args.Args; @@ -133,7 +133,7 @@ public String toString() { ByteString address = helloMessage.getAddress(); if (!address.isEmpty()) { builder.append("address:") - .append(StringUtil.encode58Check(address.toByteArray())).append("\n"); + .append(WalletUtil.getAddressString(address.toByteArray())).append("\n"); } ByteString signature = helloMessage.getSignature(); 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 e9aa801f078..5a021e1f2d9 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 @@ -862,6 +862,9 @@ static String escapeBytes(ByteString input, final String fieldName, boolean self static String escapeBytesSelfType(ByteString input, final String fieldName) { //Address if (HttpSelfFormatFieldName.isAddressFormat(fieldName)) { + if (input.size() > Constant.TRON_ADDRESS_SIZE) { + return ByteArray.toHexString(input.toByteArray()); + } return StringUtil.encode58Check(input.toByteArray()); } //Normal String diff --git a/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java b/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java new file mode 100644 index 00000000000..92e5e968e24 --- /dev/null +++ b/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java @@ -0,0 +1,67 @@ +package org.tron.common.utils; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mockStatic; + +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.Collections; +import org.apache.commons.lang3.StringUtils; +import org.junit.Test; +import org.mockito.MockedStatic; + +public class WalletUtilTest { + + @Test + public void encodesAddressSizesAsBase58AndDoesNotModifyInput() { + int[] sizes = {20, 21}; + String[] expected = {"6x36TdFVhCQBxJz1MzEoySdejiLcwZMed", + "TFvF6WzyeRKMM8NcbddxL88mZXTeuUWA8Z"}; + for (int i = 0; i < sizes.length; i++) { + byte[] input = bytes(sizes[i]); + byte[] original = input.clone(); + assertEquals(expected[i], StringUtil.encode58Check(input)); + assertEquals(expected[i], WalletUtil.getAddressString(input)); + assertArrayEquals(original, input); + } + } + + @Test + public void formatsShortInputsAsFullHex() { + assertEquals("invalid-address(,len=0)", WalletUtil.getAddressString(bytes(0))); + assertEquals("invalid-address(41,len=1)", WalletUtil.getAddressString(bytes(1))); + assertEquals("invalid-address(" + StringUtils.repeat("41", 19) + ",len=19)", + WalletUtil.getAddressString(bytes(19))); + } + + @Test + public void formatsOversizedInputsWithoutHashingOrBase58Conversion() { + for (int size : new int[]{22, 4096, 16384, 65536}) { + byte[] input = bytes(size); + try (MockedStatic hash = mockStatic(Sha256Hash.class); + MockedStatic base58 = mockStatic(Base58.class)) { + String expected = "invalid-address(" + StringUtils.repeat("41", 21) + + "...,len=" + size + ")"; + assertEquals(expected, WalletUtil.getAddressString(input)); + hash.verifyNoInteractions(); + base58.verifyNoInteractions(); + } + } + } + + @Test + public void preservesOrdinaryWitnessLists() { + ByteString address = ByteString.copyFrom(bytes(21)); + assertEquals(Collections.nCopies(27, StringUtil.encode58Check(address.toByteArray())), + WalletUtil.getAddressStringList(Collections.nCopies(27, address))); + assertTrue(WalletUtil.getAddressStringList(Collections.emptyList()).isEmpty()); + } + + private static byte[] bytes(int size) { + byte[] input = new byte[size]; + Arrays.fill(input, (byte) 0x41); + return input; + } +} diff --git a/framework/src/test/java/org/tron/core/db/Base58ValidationErrorTest.java b/framework/src/test/java/org/tron/core/db/Base58ValidationErrorTest.java new file mode 100644 index 00000000000..2db0a0c68f0 --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/Base58ValidationErrorTest.java @@ -0,0 +1,53 @@ +package org.tron.core.db; + +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import com.google.protobuf.ByteString; +import org.apache.commons.lang3.StringUtils; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.common.runtime.Runtime; +import org.tron.common.utils.Base58; +import org.tron.core.ChainBaseManager; +import org.tron.core.capsule.TransactionCapsule; +import org.tron.core.exception.ContractValidateException; +import org.tron.core.store.AccountStore; +import org.tron.core.store.ContractStore; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.core.store.StoreFactory; +import org.tron.protos.Protocol.Transaction.Contract.ContractType; +import org.tron.protos.contract.SmartContractOuterClass.TriggerSmartContract; + +public class Base58ValidationErrorTest { + + @Test + public void missingContractKeepsContractValidationExceptionAndConstantinopleGate() + throws Exception { + ChainBaseManager manager = mock(ChainBaseManager.class); + DynamicPropertiesStore properties = mock(DynamicPropertiesStore.class); + when(properties.getLatestVersion()).thenReturn(1); + when(manager.getDynamicPropertiesStore()).thenReturn(properties); + when(manager.getContractStore()).thenReturn(mock(ContractStore.class)); + when(manager.getAccountStore()).thenReturn(mock(AccountStore.class)); + StoreFactory stores = mock(StoreFactory.class); + when(stores.getChainBaseManager()).thenReturn(manager); + TransactionCapsule transaction = new TransactionCapsule(TriggerSmartContract.newBuilder() + .setContractAddress(ByteString.copyFrom(new byte[16384])).build(), + ContractType.TriggerSmartContract); + TransactionTrace trace = new TransactionTrace(transaction, stores, mock(Runtime.class)); + try (MockedStatic base58 = mockStatic(Base58.class)) { + ContractValidateException failure = assertThrows(ContractValidateException.class, + trace::checkIsConstant); + assertTrue(failure.getMessage(), + failure.getMessage().contains( + "invalid-address(" + StringUtils.repeat("00", 21) + "...,len=16384)")); + when(properties.getAllowTvmConstantinople()).thenReturn(1L); + trace.checkIsConstant(); + base58.verifyNoInteractions(); + } + } +} diff --git a/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java b/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java new file mode 100644 index 00000000000..4352e500ff0 --- /dev/null +++ b/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java @@ -0,0 +1,214 @@ +package org.tron.core.net; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.AppenderBase; +import com.google.protobuf.ByteString; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.net.InetSocketAddress; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.slf4j.LoggerFactory; +import org.tron.common.overlay.message.Message; +import org.tron.common.utils.Base58; +import org.tron.common.utils.StringUtil; +import org.tron.consensus.base.Param; +import org.tron.consensus.base.PbftInterface; +import org.tron.consensus.pbft.message.PbftMessage; +import org.tron.core.net.message.MessageTypes; +import org.tron.core.net.message.handshake.HelloMessage; +import org.tron.core.net.messagehandler.PbftMsgHandler; +import org.tron.core.net.peer.PeerConnection; +import org.tron.core.net.peer.PeerManager; +import org.tron.core.net.service.handshake.HandshakeService; +import org.tron.core.net.service.statistics.PeerStatistics; +import org.tron.p2p.P2pConfig; +import org.tron.p2p.P2pService; +import org.tron.p2p.base.Parameter; +import org.tron.p2p.connection.Channel; +import org.tron.protos.Discover; +import org.tron.protos.Protocol; +import org.tron.protos.Protocol.PBFTMessage; +import org.tron.protos.Protocol.ReasonCode; + +public class Base58MessageLoggingTest { + + private final Logger logger = (Logger) LoggerFactory.getLogger("net"); + private final List logMessages = new ArrayList<>(); + private Level oldLevel; + private P2pConfig oldConfig; + private PbftInterface oldPbftInterface; + private AppenderBase appender; + + @Before + public void setUp() { + oldLevel = logger.getLevel(); + logger.setLevel(Level.INFO); + appender = new AppenderBase() { + @Override + protected void append(ILoggingEvent event) { + // Force formatting inside the receive path, as a normal logging appender does. + logMessages.add(event.getFormattedMessage()); + } + }; + appender.start(); + logger.addAppender(appender); + oldConfig = Parameter.p2pConfig; + Parameter.p2pConfig = new P2pConfig(); + Parameter.p2pConfig.setIp("127.0.0.1"); + oldPbftInterface = Param.getInstance().getPbftInterface(); + Param.getInstance().setPbftInterface(mock(PbftInterface.class)); + } + + @After + public void tearDown() { + logger.detachAppender(appender); + appender.stop(); + logger.setLevel(oldLevel); + Parameter.p2pConfig = oldConfig; + Param.getInstance().setPbftInterface(oldPbftInterface); + } + + @Test + public void helloReceiveLogCannotEncodeOversizedAddressBeforeHandshakeRejection() + throws Exception { + for (Level level : new Level[]{Level.INFO, Level.OFF}) { + logger.setLevel(level); + logMessages.clear(); + PeerConnection peer = peer(); + P2pEventHandlerImpl handler = new P2pEventHandlerImpl(); + setField(handler, "handshakeService", new HandshakeService()); + Method process = P2pEventHandlerImpl.class.getDeclaredMethod("processMessage", + PeerConnection.class, byte[].class); + process.setAccessible(true); + HelloMessage message = hello(16384); + try (MockedStatic network = mockStatic(TronNetService.class); + MockedStatic base58 = mockStatic(Base58.class)) { + network.when(TronNetService::getP2pService).thenReturn(mock(P2pService.class)); + process.invoke(handler, peer, message.getSendBytes()); + verify(peer).disconnect(ReasonCode.INCOMPATIBLE_PROTOCOL); + base58.verifyNoInteractions(); + } + if (level == Level.INFO) { + assertTrue(logMessages.stream().anyMatch(s -> s.contains(",len=16384)"))); + } + } + } + + @Test + public void helloFormattingPreservesSmallInputsAndExistingLengthValidation() throws Exception { + HelloMessage empty = hello(0); + assertTrue(empty.valid()); + assertFalse(empty.toString().contains("address:")); + HelloMessage normal = hello(21); + assertTrue(normal.valid()); + assertTrue(normal.toString().contains(StringUtil.encode58Check(bytes(21).toByteArray()))); + HelloMessage unusual = hello(200); + assertTrue(unusual.valid()); + assertTrue(unusual.toString().contains(",len=200)")); + assertFalse(hello(201).valid()); + } + + @Test + public void pbftSignatureFailureLogIsSafeAndDisabledPbftStillReturnsEarly() throws Exception { + PbftMessage message = pbft(Protocol.SRL.newBuilder() + .addSrAddress(bytes(16384)).build().toByteString()); + for (boolean enabled : new boolean[]{false, true}) { + PeerConnection peer = peer(); + Channel channel = peer.getChannel(); + P2pEventHandlerImpl handler = new P2pEventHandlerImpl(); + PbftMsgHandler pbftHandler = new PbftMsgHandler(); + TronNetDelegate delegate = mock(TronNetDelegate.class); + when(delegate.allowPBFT()).thenReturn(enabled); + setField(pbftHandler, "tronNetDelegate", delegate); + setField(handler, "pbftMsgHandler", pbftHandler); + try (MockedStatic peers = mockStatic(PeerManager.class); + MockedStatic messages = mockStatic(Message.class, CALLS_REAL_METHODS); + MockedStatic base58 = mockStatic(Base58.class)) { + peers.when(() -> PeerManager.getPeerConnection(channel)).thenReturn(peer); + messages.when(Message::isFilter).thenReturn(false); + logMessages.clear(); + handler.onMessage(channel, message.getSendBytes()); + if (enabled) { + verify(peer).disconnect(ReasonCode.BAD_PROTOCOL); + assertTrue(logMessages.stream().anyMatch(s -> s.contains(",len=16384)"))); + } else { + verify(peer, never()).disconnect(ReasonCode.BAD_PROTOCOL); + assertTrue(logMessages.toString(), logMessages.isEmpty()); + } + base58.verifyNoInteractions(); + } + } + } + + @Test + public void formatsSrlListAndHandlesMalformedSrl() { + ByteString address = bytes(21); + PbftMessage ordinary = pbft(Protocol.SRL.newBuilder() + .addAllSrAddress(Collections.nCopies(27, address)).build().toByteString()); + assertEquals("sr list = " + + Collections.nCopies(27, StringUtil.encode58Check(address.toByteArray())), + ordinary.getDataString()); + assertEquals("decode error", pbft(ByteString.copyFrom(new byte[]{-1})).getDataString()); + } + + private static PeerConnection peer() { + PeerConnection peer = mock(PeerConnection.class); + when(peer.getPeerStatistics()).thenReturn(new PeerStatistics()); + when(peer.getChannel()).thenReturn(mock(Channel.class)); + when(peer.getInetSocketAddress()).thenReturn(new InetSocketAddress("127.0.0.1", 18888)); + return peer; + } + + private static HelloMessage hello(int size) throws Exception { + Protocol.HelloMessage.BlockId block = Protocol.HelloMessage.BlockId.newBuilder() + .setHash(ByteString.copyFrom(new byte[32])).build(); + Protocol.HelloMessage value = Protocol.HelloMessage.newBuilder() + .setFrom(Discover.Endpoint.newBuilder().setAddress(ByteString.copyFromUtf8("127.0.0.1")) + .setPort(18888).setNodeId(ByteString.copyFrom(new byte[64]))) + .setGenesisBlockId(block).setSolidBlockId(block).setHeadBlockId(block) + .setAddress(bytes(size)).build(); + return new HelloMessage(value.toByteArray()); + } + + private static PbftMessage pbft(ByteString data) { + PbftMessage message = new PbftMessage(); + PBFTMessage value = PBFTMessage.newBuilder().setRawData(PBFTMessage.Raw.newBuilder() + .setDataType(PBFTMessage.DataType.SRL).setMsgType(PBFTMessage.MsgType.PREPREPARE) + .setData(data)).build(); + message.setPbftMessage(value); + message.setData(value.toByteArray()); + message.setType(MessageTypes.PBFT_MSG.asByte()); + return message; + } + + private static ByteString bytes(int size) { + byte[] value = new byte[size]; + Arrays.fill(value, (byte) 0x41); + return ByteString.copyFrom(value); + } + + private static void setField(Object target, String name, Object value) throws Exception { + Field field = target.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(target, value); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/BroadcastHexAddressValidationTest.java b/framework/src/test/java/org/tron/core/services/http/BroadcastHexAddressValidationTest.java new file mode 100644 index 00000000000..fe33b7d436d --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/BroadcastHexAddressValidationTest.java @@ -0,0 +1,125 @@ +package org.tron.core.services.http; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import java.io.BufferedReader; +import java.io.PrintWriter; +import java.io.StringReader; +import java.io.StringWriter; +import java.lang.reflect.Field; +import java.util.Arrays; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI.Return; +import org.tron.api.GrpcAPI.Return.response_code; +import org.tron.common.utils.Base58; +import org.tron.common.utils.ByteArray; +import org.tron.common.utils.StringUtil; +import org.tron.core.Wallet; +import org.tron.json.JSONObject; +import org.tron.protos.Protocol; +import org.tron.protos.Protocol.Transaction; +import org.tron.protos.contract.BalanceContract.TransferContract; + +public class BroadcastHexAddressValidationTest { + + @Test + public void rendersOversizedAuthAddressAsHexWithoutBase58Conversion() throws Exception { + for (int size : new int[]{22, 4096, 16384, 65536}) { + Wallet wallet = mock(Wallet.class); + Transaction transaction = transaction(size); + when(wallet.broadcastTransaction(transaction)).thenReturn(Return.newBuilder() + .setResult(false).setCode(response_code.CONTRACT_VALIDATE_ERROR).build()); + try (MockedStatic base58 = mockStatic(Base58.class)) { + JSONObject result = broadcast(transaction, wallet); + assertEquals(response_code.CONTRACT_VALIDATE_ERROR.name(), result.getString("code")); + assertTrue(result.getString("transaction") + .contains(ByteArray.toHexString(bytes(size).toByteArray()))); + base58.verifyNoInteractions(); + } + } + } + + @Test + public void preservesSuccessfulAndRejectedBroadcastResponses() throws Exception { + Transaction transaction = transaction(21); + for (response_code code : new response_code[]{response_code.SUCCESS, response_code.SIGERROR}) { + Wallet wallet = mock(Wallet.class); + boolean success = code == response_code.SUCCESS; + Return result = Return.newBuilder().setResult(success).setCode(code) + .setMessage(ByteString.copyFromUtf8("test result")).build(); + when(wallet.broadcastTransaction(transaction)).thenReturn(result); + JSONObject response = broadcast(transaction, wallet); + assertEquals(success, response.getBoolean("result")); + assertEquals(code.name(), response.getString("code")); + assertEquals("test result", response.getString("message")); + assertEquals(JsonFormat.printToString(transaction, true), response.getString("transaction")); + assertEquals(64, response.getString("txid").length()); + verify(wallet).broadcastTransaction(transaction); + } + } + + @Test + public void jsonFormatterPreservesTwentyByteLogAddresses() { + ByteString address = bytes(20); + Protocol.TransactionInfo.Log log = Protocol.TransactionInfo.Log.newBuilder() + .setAddress(address).build(); + JSONObject json = JSONObject.parseObject(JsonFormat.printToString(log, true)); + assertEquals(StringUtil.encode58Check(address.toByteArray()), json.getString("address")); + assertEquals(ByteArray.toHexString(bytes(22).toByteArray()), JSONObject.parseObject( + JsonFormat.printToString(log.toBuilder().setAddress(bytes(22)).build(), true)) + .getString("address")); + assertEquals(ByteArray.toHexString(bytes(22).toByteArray()), JSONObject.parseObject( + JsonFormat.printToString(log.toBuilder().setAddress(bytes(22)).build(), false)) + .getString("address")); + } + + @Test + public void rawAnyPayloadIsStillHexRatherThanAnExpandedAddress() { + Transaction transaction = Transaction.newBuilder().setRawData(Transaction.raw.newBuilder() + .addContract(Transaction.Contract.newBuilder() + .setType(Transaction.Contract.ContractType.TransferContract) + .setParameter(Any.pack(TransferContract.newBuilder() + .setOwnerAddress(bytes(4096)).build())))).build(); + assertEquals(JsonFormat.printToString(transaction, false), + JsonFormat.printToString(transaction, true)); + } + + private static JSONObject broadcast(Transaction transaction, Wallet wallet) throws Exception { + BroadcastHexServlet servlet = new BroadcastHexServlet(); + Field walletField = BroadcastHexServlet.class.getDeclaredField("wallet"); + walletField.setAccessible(true); + walletField.set(servlet, wallet); + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + JSONObject body = new JSONObject(); + body.put("transaction", ByteArray.toHexString(transaction.toByteArray())); + when(request.getReader()).thenReturn(new BufferedReader(new StringReader(body.toJSONString()))); + StringWriter output = new StringWriter(); + when(response.getWriter()).thenReturn(new PrintWriter(output)); + servlet.doPost(request, response); + return JSONObject.parseObject(output.toString()); + } + + private static Transaction transaction(int size) { + return Transaction.newBuilder().setRawData(Transaction.raw.newBuilder() + .addAuths(Protocol.authority.newBuilder().setAccount(Protocol.AccountId.newBuilder() + .setAddress(bytes(size))))) + .addSignature(ByteString.copyFrom(new byte[]{1})).build(); + } + + private static ByteString bytes(int size) { + byte[] input = new byte[size]; + Arrays.fill(input, (byte) 0x41); + return ByteString.copyFrom(input); + } +} From 6142b03a38b7a5f68244b24da48bf2d3cd60c92e Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 29 Sep 2026 14:43:46 +0800 Subject: [PATCH 07/14] fix(api): surface input-validation messages past the error sanitizer The response sanitizer forwards only a whitelist of exception types and turns the rest into "internal server error", which hid the fixed texts of HTTP parameter validation. Those checks now throw InvalidHttpParameterException, which the sanitizer forwards; java.security.InvalidParameterException stays off the whitelist because the JDK and libraries throw it too. The checks used only by triggersmartcontract, triggerconstantcontract and estimateenergy keep InvalidParameterException, since those servlets format their own errors. --- .../InvalidHttpParameterException.java | 15 +++++++ .../org/tron/core/services/http/Util.java | 43 +++++++++++-------- .../http/GetExchangeByIdServletTest.java | 2 +- .../http/JsonLongValueValidationTest.java | 28 ++++++++---- .../services/http/UtilProcessErrorTest.java | 6 +++ 5 files changed, 66 insertions(+), 28 deletions(-) create mode 100644 common/src/main/java/org/tron/core/exception/InvalidHttpParameterException.java diff --git a/common/src/main/java/org/tron/core/exception/InvalidHttpParameterException.java b/common/src/main/java/org/tron/core/exception/InvalidHttpParameterException.java new file mode 100644 index 00000000000..aaf13c50a36 --- /dev/null +++ b/common/src/main/java/org/tron/core/exception/InvalidHttpParameterException.java @@ -0,0 +1,15 @@ +package org.tron.core.exception; + +import java.security.InvalidParameterException; + +public class InvalidHttpParameterException extends InvalidParameterException { + + public InvalidHttpParameterException(String message) { + super(message); + } + + public InvalidHttpParameterException(String message, Throwable cause) { + super(message); + initCause(cause); + } +} diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index 2c0dd1cd155..8d9540e13fb 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -50,6 +50,7 @@ import org.tron.core.config.args.Args; import org.tron.core.db.TransactionTrace; import org.tron.core.exception.ContractValidateException; +import org.tron.core.exception.InvalidHttpParameterException; import org.tron.core.exception.MaintenanceUnavailableException; import org.tron.core.services.http.JsonFormat.ParseException; import org.tron.json.JSON; @@ -143,7 +144,8 @@ private static String clientMessage(Exception e) { } if (type == ParseException.class || type == ContractValidateException.class - || type == MaintenanceUnavailableException.class) { + || type == MaintenanceUnavailableException.class + || type == InvalidHttpParameterException.class) { String message = e.getMessage(); return StringUtils.isBlank(message) ? INTERNAL_SERVER_ERROR : message; } @@ -453,7 +455,7 @@ public static String getHexString(final String string) { * be turned away by type rather than by length. An absent value is left to the caller, which is * what distinguishes an optional key from a required one. * - * @throws InvalidParameterException if the value is not a number, or is longer than the bound + * @throws InvalidHttpParameterException if the value is not a number, or is longer than the bound */ private static void checkJsonNumberValue(Object rawValue, String message) { if (rawValue == null) { @@ -461,7 +463,7 @@ private static void checkJsonNumberValue(Object rawValue, String message) { } if (!(rawValue instanceof String || rawValue instanceof Number) || rawValue.toString().length() > MAX_JSON_INTEGER_VALUE_LENGTH) { - throw new InvalidParameterException(message); + throw new InvalidHttpParameterException(message); } } @@ -476,17 +478,17 @@ public static Transaction setTransactionPermissionId(JSONObject jsonObject, checkJsonNumberValue(rawValue, INVALID_PERMISSION_ID); BigDecimal value = jsonObject.getBigDecimal(PERMISSION_ID); if (value == null) { - throw new InvalidParameterException(INVALID_PERMISSION_ID); + throw new InvalidHttpParameterException(INVALID_PERMISSION_ID); } // Preserve getInteger's legacy string syntax, but require it to match the exact conversion. int exact = value.intValueExact(); Integer legacy = jsonObject.getInteger(PERMISSION_ID); if (legacy == null || legacy != exact) { - throw new InvalidParameterException(INVALID_PERMISSION_ID); + throw new InvalidHttpParameterException(INVALID_PERMISSION_ID); } permissionId = exact; } catch (NumberFormatException | ArithmeticException | JSONException e) { - throw new InvalidParameterException(INVALID_PERMISSION_ID); + throw new InvalidHttpParameterException(INVALID_PERMISSION_ID); } return setTransactionPermissionId(permissionId, transaction); } @@ -555,14 +557,17 @@ public static long getJsonLongValue(final JSONObject jsonObject, final String ke } public static long getJsonLongValue(JSONObject jsonObject, String key, boolean required) { - Object rawValue = jsonObject.get(key); - checkJsonNumberValue(rawValue, "invalid key [" + key + "]: expect a number of at most " - + MAX_JSON_INTEGER_VALUE_LENGTH + " characters"); - BigDecimal bigDecimal = jsonObject.getBigDecimal(key); - if (required && bigDecimal == null) { - throw new InvalidParameterException("key [" + key + "] does not exist"); + String invalid = "invalid key [" + key + "]: expect a 64-bit integer"; + try { + checkJsonNumberValue(jsonObject.get(key), invalid); + BigDecimal bigDecimal = jsonObject.getBigDecimal(key); + if (required && bigDecimal == null) { + throw new InvalidHttpParameterException("key [" + key + "] does not exist"); + } + return (bigDecimal == null) ? 0L : bigDecimal.longValueExact(); + } catch (NumberFormatException | ArithmeticException | JSONException e) { + throw new InvalidHttpParameterException(invalid, e); } - return (bigDecimal == null) ? 0L : bigDecimal.longValueExact(); } public static String getMemo(byte[] memo) { @@ -627,11 +632,13 @@ public static void printAccount(Account reply, HttpServletResponse response, Boo } /** - * Returns the address the request carries. Every way the request can fail to name one is - * reported as an IllegalArgumentException whose message is the fixed text the caller is - * answered with, so that the address-keyed endpoints and their solidity/PBFT mirrors answer a - * given malformed request identically without each having to classify the failure. Nothing - * derived from the request may go into that message: it is written straight to the response. + * Returns the address the request carries. An address parameter that is missing or unusable, + * and a json body that cannot be parsed, are reported as an IllegalArgumentException whose + * message is the fixed text the caller is answered with, so that the address-keyed endpoints + * and their solidity/PBFT mirrors answer a given malformed request identically without each + * having to classify the failure. Nothing derived from the request may go into that message: + * it is written straight to the response. A query string or form body the container itself + * cannot parse is not classified here and propagates unchanged. */ public static byte[] getAddress(HttpServletRequest request) throws Exception { String addressParam = "address"; diff --git a/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java b/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java index 51cfcbda108..b003e5e3d61 100644 --- a/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/GetExchangeByIdServletTest.java @@ -61,7 +61,7 @@ public void oversizedQuotedIdIsRejectedBeforeWalletCall() throws Exception { verifyNoInteractions(wallet); assertTrue(response.getContentAsString().contains("id")); - assertTrue(response.getContentAsString().contains("64 characters")); + assertTrue(response.getContentAsString().contains("64-bit integer")); assertFalse(response.getContentAsString().contains(oversized)); } } diff --git a/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java b/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java index 09211d99e27..2e1ec6fb8cb 100644 --- a/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java +++ b/framework/src/test/java/org/tron/core/services/http/JsonLongValueValidationTest.java @@ -2,12 +2,15 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import java.security.InvalidParameterException; import org.apache.commons.lang3.StringUtils; import org.junit.Test; +import org.tron.core.exception.InvalidHttpParameterException; import org.tron.json.JSONObject; public class JsonLongValueValidationTest { @@ -35,14 +38,16 @@ public void stringLengthBoundaryIsEnforcedBeforeDecimalConversion() { /** An unquoted token reaches this check as an already-normalized Number. */ @Test public void unquotedNumberNormalizedTextLengthBoundaryIsEnforced() { - // json forbids leading zeros, so the accepted case is shown by the failure it reaches - // instead: at 64 characters the bound is passed and the exact conversion is what rejects it - assertThrows(ArithmeticException.class, + // no 64-character normalized number fits in a long, so the accepted side of the bound is + // shown by where the rejection comes from: past the bound, the exact conversion refuses it + InvalidHttpParameterException converted = assertThrows(InvalidHttpParameterException.class, () -> Util.getJsonLongValue(unquoted(StringUtils.repeat('9', 64)), "id", true)); + assertTrue(converted.getCause() instanceof ArithmeticException); String oversized = StringUtils.repeat('9', 65); - InvalidParameterException exception = assertThrows(InvalidParameterException.class, + InvalidHttpParameterException exception = assertThrows(InvalidHttpParameterException.class, () -> Util.getJsonLongValue(unquoted(oversized), "id", true)); + assertNull(exception.getCause()); assertTrue(exception.getMessage().contains("id")); assertFalse(exception.getMessage().contains(oversized)); } @@ -59,11 +64,16 @@ public void exactLongRepresentationsRemainCompatible() { } @Test - public void existingExactConversionFailuresRemainInEffect() { - assertThrows(ArithmeticException.class, - () -> Util.getJsonLongValue(value("1.5"), "id", true)); - assertThrows(ArithmeticException.class, - () -> Util.getJsonLongValue(value("9223372036854775808"), "id", true)); + public void conversionFailuresAreReportedAsInvalidParameter() { + String[] rawJsonValues = {"\"1.5\"", "1.5", "\"9223372036854775808\"", "\"abc\""}; + for (String rawJsonValue : rawJsonValues) { + JSONObject jsonObject = JSONObject.parseObject("{\"id\":" + rawJsonValue + "}"); + InvalidHttpParameterException exception = assertThrows( + InvalidHttpParameterException.class, + () -> Util.getJsonLongValue(jsonObject, "id", true)); + assertEquals("invalid key [id]: expect a 64-bit integer", exception.getMessage()); + assertNotNull(exception.getCause()); + } } /** diff --git a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java index 5d4baa34c6f..065467ff240 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java @@ -4,12 +4,14 @@ import static org.junit.Assert.assertThrows; import com.google.protobuf.InvalidProtocolBufferException; +import java.security.InvalidParameterException; import org.bouncycastle.util.encoders.DecoderException; import org.bouncycastle.util.encoders.Hex; import org.junit.Test; import org.springframework.mock.web.MockHttpServletResponse; import org.tron.core.exception.ContractValidateException; import org.tron.core.exception.HeaderNotFound; +import org.tron.core.exception.InvalidHttpParameterException; import org.tron.core.exception.MaintenanceUnavailableException; import org.tron.core.exception.ZkProofValidateException; import org.tron.json.JSONException; @@ -28,6 +30,8 @@ public void exactCompatibilityTypesPreserveNonBlankMessage() throws Exception { "balance is not sufficient"); assertError(new MaintenanceUnavailableException("maintenance in progress"), "maintenance in progress"); + assertError(new InvalidHttpParameterException("invalid key [id]: expect a 64-bit integer"), + "invalid key [id]: expect a 64-bit integer"); } @Test @@ -65,6 +69,8 @@ public void nullBlankAndSubclassMessagesFailClosed() throws Exception { assertError(new JsonFormat.ParseException(""), INTERNAL_SERVER_ERROR); assertError(new JsonFormat.ParseException(" "), INTERNAL_SERVER_ERROR); assertError(new ContractValidateException("subclass message") { }, INTERNAL_SERVER_ERROR); + assertError(new InvalidHttpParameterException(null), INTERNAL_SERVER_ERROR); + assertError(new InvalidHttpParameterException(" "), INTERNAL_SERVER_ERROR); } @Test From ba54931cd935cb006908193d8544201794484543 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Fri, 9 Oct 2026 13:36:06 +0800 Subject: [PATCH 08/14] fix(api): simplify address and Permission_id error handling - drop the address guard from contract queries; the log line bounds the address itself - report an unusable address as InvalidHttpParameterException through processError - accept Permission_id only as a JSON int or a decimal string - keep the cause of a rejected Permission_id --- .../src/main/java/org/tron/core/Wallet.java | 6 - .../services/http/GetBrokerageServlet.java | 3 - .../core/services/http/GetRewardServlet.java | 3 - .../org/tron/core/services/http/Util.java | 44 +++---- .../WalletContractAddressLoggingTest.java | 41 ++++++ .../WalletContractAddressValidationTest.java | 121 ------------------ .../java/org/tron/core/WalletMockTest.java | 8 +- .../http/AddressQueryServletTestBase.java | 1 + .../services/http/TransferServletTest.java | 9 -- .../services/http/UtilProcessErrorTest.java | 1 + .../org/tron/core/services/http/UtilTest.java | 58 +++------ 11 files changed, 78 insertions(+), 217 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/WalletContractAddressLoggingTest.java delete mode 100644 framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index ed8f349b6f9..29b675bfcde 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -3205,9 +3205,6 @@ public Transaction callConstantContract(TransactionCapsule trxCap, public SmartContract getContract(GrpcAPI.BytesMessage bytesMessage) { byte[] address = bytesMessage.getValue().toByteArray(); - if (!DecodeUtil.addressValid(address)) { - return null; - } AccountCapsule accountCapsule = chainBaseManager.getAccountStore().get(address); if (accountCapsule == null) { logger.warn( @@ -3237,9 +3234,6 @@ public SmartContract getContract(GrpcAPI.BytesMessage bytesMessage) { */ public SmartContractDataWrapper getContractInfo(GrpcAPI.BytesMessage bytesMessage) { byte[] address = bytesMessage.getValue().toByteArray(); - if (!DecodeUtil.addressValid(address)) { - return null; - } AccountCapsule accountCapsule = chainBaseManager.getAccountStore().get(address); if (accountCapsule == null) { logger.warn( diff --git a/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java b/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java index e7f1aa88cbb..4bfc8831934 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java @@ -19,9 +19,6 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { byte[] address; try { address = Util.getAddress(request); - } catch (IllegalArgumentException e) { - Util.writeAuditedError(e.getMessage(), response); - return; } catch (Exception e) { Util.processError(e, response); return; diff --git a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java index d0c20153586..3235b7dd9d6 100644 --- a/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java @@ -19,9 +19,6 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { byte[] address; try { address = Util.getAddress(request); - } catch (IllegalArgumentException e) { - Util.writeAuditedError(e.getMessage(), response); - return; } catch (Exception e) { Util.processError(e, response); return; diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index 8d9540e13fb..2c5c81df29e 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -472,23 +472,18 @@ public static Transaction setTransactionPermissionId(JSONObject jsonObject, if (!jsonObject.containsKey(PERMISSION_ID)) { return transaction; } + Object rawValue = jsonObject.get(PERMISSION_ID); int permissionId; try { - Object rawValue = jsonObject.get(PERMISSION_ID); - checkJsonNumberValue(rawValue, INVALID_PERMISSION_ID); - BigDecimal value = jsonObject.getBigDecimal(PERMISSION_ID); - if (value == null) { - throw new InvalidHttpParameterException(INVALID_PERMISSION_ID); - } - // Preserve getInteger's legacy string syntax, but require it to match the exact conversion. - int exact = value.intValueExact(); - Integer legacy = jsonObject.getInteger(PERMISSION_ID); - if (legacy == null || legacy != exact) { + if (rawValue instanceof Integer) { + permissionId = (Integer) rawValue; + } else if (rawValue instanceof String) { + permissionId = Integer.parseInt((String) rawValue); + } else { throw new InvalidHttpParameterException(INVALID_PERMISSION_ID); } - permissionId = exact; - } catch (NumberFormatException | ArithmeticException | JSONException e) { - throw new InvalidHttpParameterException(INVALID_PERMISSION_ID); + } catch (NumberFormatException e) { + throw new InvalidHttpParameterException(INVALID_PERMISSION_ID, e); } return setTransactionPermissionId(permissionId, transaction); } @@ -633,12 +628,11 @@ public static void printAccount(Account reply, HttpServletResponse response, Boo /** * Returns the address the request carries. An address parameter that is missing or unusable, - * and a json body that cannot be parsed, are reported as an IllegalArgumentException whose - * message is the fixed text the caller is answered with, so that the address-keyed endpoints - * and their solidity/PBFT mirrors answer a given malformed request identically without each - * having to classify the failure. Nothing derived from the request may go into that message: - * it is written straight to the response. A query string or form body the container itself - * cannot parse is not classified here and propagates unchanged. + * and a json body that cannot be parsed, are reported as an InvalidHttpParameterException whose + * message is a fixed text, so that the address-keyed endpoints and their solidity/PBFT mirrors + * answer a given malformed request identically without each having to classify the failure. + * Nothing derived from the request may go into that message. A query string or form body the + * container itself cannot parse is not classified here and propagates unchanged. */ public static byte[] getAddress(HttpServletRequest request) throws Exception { String addressParam = "address"; @@ -646,18 +640,16 @@ public static byte[] getAddress(HttpServletRequest request) throws Exception { try { addressStr = checkGetParam(request, addressParam); } catch (JSONException e) { - throw new IllegalArgumentException(INVALID_JSON_BODY); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + throw new InvalidHttpParameterException(INVALID_JSON_BODY); } if (StringUtils.isBlank(addressStr)) { - throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } boolean hex = StringUtils.startsWith(addressStr, Constant.ADD_PRE_FIX_STRING_MAINNET); // bound the hex input before decoding, mirroring the base58 length short-circuit if (hex && addressStr.length() != DecodeUtil.ADDRESS_SIZE) { - throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } byte[] address; @@ -665,11 +657,11 @@ public static byte[] getAddress(HttpServletRequest request) throws Exception { address = hex ? Hex.decode(addressStr) : decodeFromBase58Check(addressStr); } catch (DecoderException | IllegalArgumentException exception) { // both decoders name the offending character and its offset, which is caller input - throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } // base58 is validated inside the decoder; hex used to be returned unchecked if (address == null || (hex && !DecodeUtil.addressValid(address))) { - throw new IllegalArgumentException(INVALID_ADDRESS_MSG); + throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } return address; } diff --git a/framework/src/test/java/org/tron/core/WalletContractAddressLoggingTest.java b/framework/src/test/java/org/tron/core/WalletContractAddressLoggingTest.java new file mode 100644 index 00000000000..7f2ca3094b4 --- /dev/null +++ b/framework/src/test/java/org/tron/core/WalletContractAddressLoggingTest.java @@ -0,0 +1,41 @@ +package org.tron.core; + +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import com.google.protobuf.ByteString; +import java.lang.reflect.Field; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI.BytesMessage; +import org.tron.common.utils.Base58; +import org.tron.core.capsule.AccountCapsule; +import org.tron.core.store.AccountStore; + +public class WalletContractAddressLoggingTest { + + @Test + public void oversizedMissingAddressIsLoggedWithoutBase58() throws Exception { + Wallet wallet = new Wallet(); + ChainBaseManager chainBaseManager = mock(ChainBaseManager.class); + AccountStore accountStore = mock(AccountStore.class); + when(accountStore.get(any(byte[].class))).thenReturn((AccountCapsule) null); + when(chainBaseManager.getAccountStore()).thenReturn(accountStore); + Field field = Wallet.class.getDeclaredField("chainBaseManager"); + field.setAccessible(true); + field.set(wallet, chainBaseManager); + + for (int size : new int[] {22, 32 * 1024}) { + BytesMessage message = BytesMessage.newBuilder() + .setValue(ByteString.copyFrom(new byte[size])).build(); + try (MockedStatic base58 = mockStatic(Base58.class)) { + assertNull(wallet.getContract(message)); + assertNull(wallet.getContractInfo(message)); + base58.verifyNoInteractions(); + } + } + } +} diff --git a/framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java b/framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java deleted file mode 100644 index 9063bc7a0e0..00000000000 --- a/framework/src/test/java/org/tron/core/WalletContractAddressValidationTest.java +++ /dev/null @@ -1,121 +0,0 @@ -package org.tron.core; - -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.mockito.AdditionalMatchers.aryEq; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.mockStatic; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -import com.google.protobuf.ByteString; -import java.lang.reflect.Field; -import java.util.Arrays; -import org.junit.BeforeClass; -import org.junit.Test; -import org.mockito.MockedStatic; -import org.tron.api.GrpcAPI.BytesMessage; -import org.tron.common.utils.Base58; -import org.tron.core.capsule.AccountCapsule; -import org.tron.core.capsule.ContractCapsule; -import org.tron.core.store.AbiStore; -import org.tron.core.store.AccountStore; -import org.tron.core.store.ContractStore; -import org.tron.protos.contract.SmartContractOuterClass.SmartContract; - -public class WalletContractAddressValidationTest { - - // the stores are re-mocked per case, but the wallet itself carries no state these tests read; - // constructing one generates a keypair, so it is built once rather than once per address - private static Wallet wallet; - private static Field chainBaseManagerField; - - @BeforeClass - public static void setUpClass() throws Exception { - wallet = new Wallet(); - chainBaseManagerField = Wallet.class.getDeclaredField("chainBaseManager"); - chainBaseManagerField.setAccessible(true); - } - - /** - * Installs a chain base manager whose account store finds nothing, and hands back that store so - * the caller can assert whether it was ever consulted. - */ - private static AccountStore installMissingAccount() throws Exception { - ChainBaseManager chainBaseManager = mock(ChainBaseManager.class); - AccountStore accountStore = mock(AccountStore.class); - when(accountStore.get(any(byte[].class))).thenReturn((AccountCapsule) null); - when(chainBaseManager.getAccountStore()).thenReturn(accountStore); - chainBaseManagerField.set(wallet, chainBaseManager); - return accountStore; - } - - private static BytesMessage bytes(byte[] raw) { - return BytesMessage.newBuilder().setValue(ByteString.copyFrom(raw)).build(); - } - - private static byte[] canonicalAddress() { - byte[] address = new byte[21]; - Arrays.fill(address, (byte) 1); - address[0] = Wallet.getAddressPreFixByte(); - return address; - } - - @Test - public void invalidAddressesAreRejectedBeforeStorageOrBase58() throws Exception { - byte[][] invalidAddresses = { - new byte[0], - new byte[20], - new byte[22], - new byte[32 * 1024], - canonicalAddress() - }; - invalidAddresses[4][0] = (byte) (Wallet.getAddressPreFixByte() + 1); - - for (byte[] invalidAddress : invalidAddresses) { - AccountStore accountStore = installMissingAccount(); - try (MockedStatic base58 = mockStatic(Base58.class)) { - assertNull(wallet.getContract(bytes(invalidAddress))); - assertNull(wallet.getContractInfo(bytes(invalidAddress))); - verifyNoInteractions(accountStore); - base58.verifyNoInteractions(); - } - } - } - - @Test - public void canonicalMissingAddressRetainsNoResultBehavior() throws Exception { - AccountStore accountStore = installMissingAccount(); - byte[] address = canonicalAddress(); - - assertNull(wallet.getContract(bytes(address))); - assertNull(wallet.getContractInfo(bytes(address))); - - verify(accountStore, times(2)).get(aryEq(address)); - } - - @Test - public void canonicalExistingAddressRetainsContractResult() throws Exception { - ChainBaseManager chainBaseManager = mock(ChainBaseManager.class); - AccountStore accountStore = mock(AccountStore.class); - ContractStore contractStore = mock(ContractStore.class); - AbiStore abiStore = mock(AbiStore.class); - ContractCapsule contractCapsule = mock(ContractCapsule.class); - SmartContract contract = SmartContract.newBuilder().setName("existing").build(); - byte[] address = canonicalAddress(); - - when(chainBaseManager.getAccountStore()).thenReturn(accountStore); - when(chainBaseManager.getContractStore()).thenReturn(contractStore); - when(chainBaseManager.getAbiStore()).thenReturn(abiStore); - when(accountStore.get(any(byte[].class))).thenReturn(mock(AccountCapsule.class)); - when(contractStore.get(any(byte[].class))).thenReturn(contractCapsule); - when(contractCapsule.getInstance()).thenReturn(contract); - when(abiStore.get(any(byte[].class))).thenReturn(null); - chainBaseManagerField.set(wallet, chainBaseManager); - - assertSame(contract, wallet.getContract(bytes(address))); - } -} diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index fee73202b7e..e3de303c225 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -1402,10 +1402,8 @@ public void testBuildShieldedTRC20Input() throws Exception { @Test public void testGetContractInfo() throws Exception { Wallet wallet = new Wallet(); - byte[] address = new byte[21]; - address[0] = Wallet.getAddressPreFixByte(); GrpcAPI.BytesMessage bytesMessage = GrpcAPI.BytesMessage.newBuilder() - .setValue(ByteString.copyFrom(address)) + .setValue(ByteString.copyFrom("test".getBytes())) .build(); ChainBaseManager chainBaseManagerMock = mock(ChainBaseManager.class); @@ -1424,10 +1422,8 @@ public void testGetContractInfo() throws Exception { @Test public void testGetContractInfo1() throws Exception { Wallet wallet = new Wallet(); - byte[] address = new byte[21]; - address[0] = Wallet.getAddressPreFixByte(); GrpcAPI.BytesMessage bytesMessage = GrpcAPI.BytesMessage.newBuilder() - .setValue(ByteString.copyFrom(address)) + .setValue(ByteString.copyFrom("test".getBytes())) .build(); ChainBaseManager chainBaseManagerMock = mock(ChainBaseManager.class); diff --git a/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java b/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java index 9e6e218e995..ddedeb3aa1e 100644 --- a/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java +++ b/framework/src/test/java/org/tron/core/services/http/AddressQueryServletTestBase.java @@ -149,6 +149,7 @@ public void missingOrBlankAddressIsRejected() throws Exception { assertInvalid(formRequest(null), null); assertInvalid(formRequest(""), null); assertInvalid(jsonRequest(null), null); + assertInvalid(jsonRequest(""), null); } @Test diff --git a/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java b/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java index 67863a53f63..f88255db4fa 100644 --- a/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/TransferServletTest.java @@ -113,15 +113,6 @@ public void testOutOfRangePermissionIdIsRejected() throws Exception { assertFalse("must not echo the submitted value", content.contains("4294967297")); } - @Test - public void testQuotedScientificPermissionIdIsRejected() throws Exception { - String content = post("\"1e2\"").getContentAsString(); - - assertTrue("must report an error", content.contains("\"Error\"")); - assertFalse("must not hand back a transaction", content.contains("txID")); - assertFalse(content.contains("\"raw_data\"")); - } - /** * An explicit null used to reach getInteger(), whose null return blew up on unboxing with a * bare NullPointerException. It stays an error -- writing the key states intent, so a null diff --git a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java index 065467ff240..f503327a649 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilProcessErrorTest.java @@ -44,6 +44,7 @@ public void unclassifiedTypesFailClosed() throws Exception { decoder, new HeaderNotFound("latest block not found"), new IllegalArgumentException("No enum constant internal.Type.VALUE"), + new InvalidParameterException("base parameter detail"), new IllegalAccessException(RATE_LIMITER_ERROR_MSG), new IllegalAccessException("other access failure"), new ZkProofValidateException("wrapped validation detail", true) diff --git a/framework/src/test/java/org/tron/core/services/http/UtilTest.java b/framework/src/test/java/org/tron/core/services/http/UtilTest.java index d31a43afe37..57108d7d6e8 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilTest.java @@ -2,7 +2,6 @@ import com.google.protobuf.ByteString; import java.security.InvalidParameterException; -import java.util.Arrays; import javax.annotation.Resource; import org.apache.commons.lang3.StringUtils; import org.junit.Assert; @@ -309,10 +308,6 @@ public void testPrintSignWeightTooManySigsHttpPath() { .contains("too many signatures")); } - /** - * A container is not a number. It used to skip the length bound altogether and reach the - * conversion as its full serialized form, bounded only by the shim's 65535-character fallback. - */ @Test public void testPermissionIdRejectsNonNumericTypes() { assertRejected("[" + StringUtils.repeat('9', 128) + "]"); @@ -331,39 +326,9 @@ private int permissionIdOf(Transaction transaction) { } @Test - public void testPermissionIdAcceptsLegacyExactIntegerRepresentations() { + public void testPermissionIdAcceptsExactIntegerRepresentations() { Assert.assertEquals(2, permissionIdOf(applyPermissionId("2"))); Assert.assertEquals(2, permissionIdOf(applyPermissionId("\"2\""))); - Assert.assertEquals(1, permissionIdOf(applyPermissionId("1.0"))); - Assert.assertEquals(100, permissionIdOf(applyPermissionId("1e2"))); - Assert.assertEquals(1, permissionIdOf(applyPermissionId("\"1.0\""))); - Assert.assertEquals(1, permissionIdOf(applyPermissionId("\"1.\""))); - Assert.assertEquals(1000, permissionIdOf(applyPermissionId("\"1,000\""))); - } - - @Test - public void testPermissionIdAcceptsMaximumLengthNumericString() { - char[] digits = new char[64]; - Arrays.fill(digits, '0'); - digits[digits.length - 1] = '2'; - - Assert.assertEquals(2, permissionIdOf(applyPermissionId("\"" + new String(digits) + "\""))); - } - - /** - * One character past the bound is refused, and so is a value far beyond it: the field is - * bounded by its own length rather than by whatever the numeric conversion happens to survive. - */ - @Test - public void testPermissionIdRejectsStringsPastTheLengthBoundary() { - assertRejected("\"" + StringUtils.repeat('0', 64) + "2\""); - assertRejected("\"" + StringUtils.repeat('9', 10_000) + "\""); - } - - /** Quoting must not decide which regime applies: the same digits unquoted are refused too. */ - @Test - public void testPermissionIdRejectsUnquotedNumbersPastTheLengthBoundary() { - assertRejected("9" + StringUtils.repeat('9', 64)); } private void assertRejected(String rawJsonValue) { @@ -375,22 +340,29 @@ private void assertRejected(String rawJsonValue) { } @Test - public void testPermissionIdRejectsFraction() { + public void testPermissionIdRejectsNonIntegerNumbers() { assertRejected("1.9"); assertRejected("2.999"); - } - - @Test - public void testPermissionIdRejectsNonLegacyNumericStrings() { + assertRejected("1.0"); + assertRejected("1e2"); assertRejected("\"1e2\""); - assertRejected("\"1E2\""); - assertRejected("\".0\""); + assertRejected("\"1.9\""); + assertRejected("\"2.5\""); + assertRejected("\"1.90\""); + assertRejected("\"1.0\""); + assertRejected("\"1.\""); + assertRejected("\"1,000\""); } @Test public void testPermissionIdRejectsIntOverflow() { assertRejected("4294967297"); assertRejected("99999999999"); + assertRejected("\"4294967297\""); + assertRejected("\"2147483648\""); + assertRejected("\"-2147483649\""); + assertRejected("9" + StringUtils.repeat('9', 64)); + assertRejected("\"" + StringUtils.repeat('9', 10_000) + "\""); } @Test From d4d27d0178e11b65716632cec1907aa71599d0b7 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Mon, 12 Oct 2026 18:29:02 +0800 Subject: [PATCH 09/14] refactor(api): rename hex to isHex in getAddress --- .../src/main/java/org/tron/core/services/http/Util.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/framework/src/main/java/org/tron/core/services/http/Util.java b/framework/src/main/java/org/tron/core/services/http/Util.java index 2c5c81df29e..f86cda3e590 100644 --- a/framework/src/main/java/org/tron/core/services/http/Util.java +++ b/framework/src/main/java/org/tron/core/services/http/Util.java @@ -646,21 +646,21 @@ public static byte[] getAddress(HttpServletRequest request) throws Exception { throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } - boolean hex = StringUtils.startsWith(addressStr, Constant.ADD_PRE_FIX_STRING_MAINNET); + boolean isHex = StringUtils.startsWith(addressStr, Constant.ADD_PRE_FIX_STRING_MAINNET); // bound the hex input before decoding, mirroring the base58 length short-circuit - if (hex && addressStr.length() != DecodeUtil.ADDRESS_SIZE) { + if (isHex && addressStr.length() != DecodeUtil.ADDRESS_SIZE) { throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } byte[] address; try { - address = hex ? Hex.decode(addressStr) : decodeFromBase58Check(addressStr); + address = isHex ? Hex.decode(addressStr) : decodeFromBase58Check(addressStr); } catch (DecoderException | IllegalArgumentException exception) { // both decoders name the offending character and its offset, which is caller input throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } // base58 is validated inside the decoder; hex used to be returned unchecked - if (address == null || (hex && !DecodeUtil.addressValid(address))) { + if (address == null || (isHex && !DecodeUtil.addressValid(address))) { throw new InvalidHttpParameterException(INVALID_ADDRESS_MSG); } return address; From fd04799e3671874004c51cd1ae2a985e9e89af6e Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Mon, 12 Oct 2026 20:01:00 +0800 Subject: [PATCH 10/14] fix(shielded): drop the mint receive description count check Unlike transfer, mint has no merge loop, so an oversized receive list costs no quadratic time and its count check is left out of this change. Also keep the cause of the invalid amount error, and record only log events from the test thread in Base58MessageLoggingTest. --- framework/src/main/java/org/tron/core/Wallet.java | 2 +- .../core/zen/ShieldedTRC20ParametersBuilder.java | 3 --- .../test/java/org/tron/core/WalletMockTest.java | 2 -- .../WalletShieldedDescriptionValidationTest.java | 15 --------------- .../tron/core/net/Base58MessageLoggingTest.java | 5 +++++ 5 files changed, 6 insertions(+), 21 deletions(-) diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 29b675bfcde..e80e7c80c8b 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -4336,7 +4336,7 @@ public BytesMessage getTriggerInputForShieldedTRC20Contract( try { value = getBigIntegerFromString(request.getAmount()); } catch (IllegalArgumentException e) { - throw new ContractValidateException("invalid amount"); + throw new ContractValidateException("invalid amount", e); } checkBigIntegerRange(value); byte[] transparentToAddress = request.getTransparentToAddress().toByteArray(); diff --git a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java index 1a12068e924..e0792980c9f 100644 --- a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java +++ b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java @@ -402,9 +402,6 @@ private String mintParamsToHexString(GrpcAPI.ShieldedTRC20Parameters mintParams, if (value.compareTo(BigInteger.ZERO) <= 0) { throw new IllegalArgumentException("require the value be positive"); } - if (mintParams.getReceiveDescriptionCount() != 1) { - throw new IllegalArgumentException("invalid mint description number"); - } ShieldContract.ReceiveDescription revDesc = mintParams.getReceiveDescription(0); byte[] zeros = new byte[12]; diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index e3de303c225..155284ab708 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -907,8 +907,6 @@ public void testGetTriggerInputForShieldedTRC20Contract1() GrpcAPI.ShieldedTRC20Parameters shieldedTRC20Parameters = GrpcAPI.ShieldedTRC20Parameters.newBuilder() .addSpendDescription(spendDescription) - // a transfer requires exactly one receive description to clear the output-arity gate; - // the rejection of a wrong count is covered by WalletShieldedDescriptionValidationTest .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) .setParameterType("transfer") .build(); diff --git a/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java b/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java index 2339459c315..96a5f0550b2 100644 --- a/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java +++ b/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java @@ -58,19 +58,4 @@ public void transferDescriptionCardinalityIsReportedAsValidationFailure() { assertEquals("invalid shielded TRC-20 trigger input: invalid transfer output number", exception.getMessage()); } - - @Test - public void mintDescriptionCardinalityIsReportedAsValidationFailure() { - GrpcAPI.ShieldedTRC20Parameters parameters = - GrpcAPI.ShieldedTRC20Parameters.newBuilder() - .setParameterType("mint") - .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) - .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) - .build(); - - ContractValidateException exception = assertThrows(ContractValidateException.class, - () -> triggerInput(parameters, "1", 0)); - assertEquals("invalid shielded TRC-20 trigger input: invalid mint description number", - exception.getMessage()); - } } diff --git a/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java b/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java index 4352e500ff0..03ca4afc950 100644 --- a/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java +++ b/framework/src/test/java/org/tron/core/net/Base58MessageLoggingTest.java @@ -53,6 +53,7 @@ public class Base58MessageLoggingTest { private final Logger logger = (Logger) LoggerFactory.getLogger("net"); private final List logMessages = new ArrayList<>(); + private final Thread testThread = Thread.currentThread(); private Level oldLevel; private P2pConfig oldConfig; private PbftInterface oldPbftInterface; @@ -65,6 +66,10 @@ public void setUp() { appender = new AppenderBase() { @Override protected void append(ILoggingEvent event) { + // Background threads left by other tests in the same JVM also log to "net". + if (Thread.currentThread() != testThread) { + return; + } // Force formatting inside the receive path, as a normal logging appender does. logMessages.add(event.getFormattedMessage()); } From d986c515942d5a875773c44eef6091ae73deb255 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 13 Oct 2026 20:23:11 +0800 Subject: [PATCH 11/14] fix(shielded): bound mint and burn receive description counts Mint takes exactly one receive description and burn at most one; extra entries used to be dropped silently. getAddressString formats null as invalid-address(null). The transfer vectors now use populated descriptions and cover both signature paths; their expected digests are the pre-change output. --- .../org/tron/common/utils/WalletUtil.java | 3 + .../zen/ShieldedTRC20ParametersBuilder.java | 6 + .../org/tron/common/utils/WalletUtilTest.java | 5 + ...lletShieldedDescriptionValidationTest.java | 67 +++++++- .../ShieldedTRC20ParametersBuilderTest.java | 148 ++++++++++-------- 5 files changed, 162 insertions(+), 67 deletions(-) diff --git a/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java b/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java index 57ef02d4cd0..f2d4af266bc 100644 --- a/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java +++ b/chainbase/src/main/java/org/tron/common/utils/WalletUtil.java @@ -120,6 +120,9 @@ public static boolean isConstant(SmartContract.ABI abi, byte[] selector) { } public static String getAddressString(byte[] address) { + if (address == null) { + return INVALID_ADDRESS + "(null)"; + } if (address.length == Constant.STANDARD_ADDRESS_SIZE || address.length == Constant.TRON_ADDRESS_SIZE) { return encode58Check(address); diff --git a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java index e0792980c9f..ea91ea6b10f 100644 --- a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java +++ b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java @@ -402,6 +402,9 @@ private String mintParamsToHexString(GrpcAPI.ShieldedTRC20Parameters mintParams, if (value.compareTo(BigInteger.ZERO) <= 0) { throw new IllegalArgumentException("require the value be positive"); } + if (mintParams.getReceiveDescriptionCount() != 1) { + throw new IllegalArgumentException("invalid mint output number"); + } ShieldContract.ReceiveDescription revDesc = mintParams.getReceiveDescription(0); byte[] zeros = new byte[12]; @@ -512,6 +515,9 @@ private String burnParamsToHexString(GrpcAPI.ShieldedTRC20Parameters burnParams, if (value.compareTo(BigInteger.ZERO) <= 0) { throw new IllegalArgumentException("the value must be positive"); } + if (burnParams.getReceiveDescriptionCount() > 1) { + throw new IllegalArgumentException("invalid burn output number"); + } byte[] transparentToAddressTvm = normalizeTransparentToAddress(transparentToAddress); diff --git a/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java b/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java index 92e5e968e24..db1e7b311b1 100644 --- a/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java +++ b/framework/src/test/java/org/tron/common/utils/WalletUtilTest.java @@ -36,6 +36,11 @@ public void formatsShortInputsAsFullHex() { WalletUtil.getAddressString(bytes(19))); } + @Test + public void formatsNullAsInvalidAddress() { + assertEquals("invalid-address(null)", WalletUtil.getAddressString(null)); + } + @Test public void formatsOversizedInputsWithoutHashingOrBase58Conversion() { for (int size : new int[]{22, 4096, 16384, 65536}) { diff --git a/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java b/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java index 96a5f0550b2..a40dbc105c5 100644 --- a/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java +++ b/framework/src/test/java/org/tron/core/WalletShieldedDescriptionValidationTest.java @@ -1,19 +1,26 @@ package org.tron.core; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; +import com.google.protobuf.ByteString; +import java.math.BigInteger; +import java.util.Arrays; import org.junit.BeforeClass; import org.junit.Test; import org.mockito.MockedStatic; import org.tron.api.GrpcAPI; import org.tron.api.GrpcAPI.BytesMessage; import org.tron.common.parameter.CommonParameter; +import org.tron.common.utils.ByteArray; +import org.tron.common.utils.ByteUtil; import org.tron.core.config.args.Args; import org.tron.core.exception.ContractValidateException; +import org.tron.core.zen.note.NoteEncryption.Encryption; import org.tron.protos.contract.ShieldContract; public class WalletShieldedDescriptionValidationTest { @@ -25,7 +32,7 @@ public static void setUpClass() { wallet = new Wallet(); } - private static void triggerInput(GrpcAPI.ShieldedTRC20Parameters parameters, + private static BytesMessage triggerInput(GrpcAPI.ShieldedTRC20Parameters parameters, String amount, int signatureCount) throws Exception { GrpcAPI.ShieldedTRC20TriggerContractParameters.Builder request = GrpcAPI.ShieldedTRC20TriggerContractParameters.newBuilder() @@ -39,10 +46,40 @@ private static void triggerInput(GrpcAPI.ShieldedTRC20Parameters parameters, try (MockedStatic mocked = mockStatic(CommonParameter.class)) { when(CommonParameter.getInstance()).thenReturn(commonParameter); when(commonParameter.isAllowShieldedTransactionApi()).thenReturn(true); - wallet.getTriggerInputForShieldedTRC20Contract(request.build()); + return wallet.getTriggerInputForShieldedTRC20Contract(request.build()); } } + private static GrpcAPI.ShieldedTRC20Parameters withReceives( + GrpcAPI.ShieldedTRC20Parameters.Builder parameters, int receives) { + for (int i = 0; i < receives; i++) { + parameters.addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()); + } + return parameters.build(); + } + + /** + * A burn whose v2 record carries the marker and the nonce bound to (nullifier, amount, zero + * transparent address), so the request clears the existing record checks. + */ + private static GrpcAPI.ShieldedTRC20Parameters burn(int receives, String amount) { + byte[] nullifier = new byte[32]; + Arrays.fill(nullifier, (byte) 1); + byte[] addr21 = new byte[21]; + addr21[0] = Wallet.getAddressPreFixByte(); + byte[] nonce = Encryption.deriveBurnNonce(nullifier, + ByteUtil.bigIntegerToBytes(new BigInteger(amount), 32), addr21); + byte[] record = new byte[Encryption.BURN_CIPHER_RECORD_SIZE]; + System.arraycopy(nonce, 0, record, Encryption.BURN_NONCE_OFFSET, Encryption.BURN_NONCE_LEN); + System.arraycopy(Encryption.getBurnRecordV2Marker(), 0, record, + Encryption.BURN_RESERVED_OFFSET, Encryption.BURN_RESERVED_LEN); + return withReceives(GrpcAPI.ShieldedTRC20Parameters.newBuilder() + .setParameterType("burn") + .setTriggerContractInput(ByteArray.toHexString(record)) + .addSpendDescription(ShieldContract.SpendDescription.newBuilder() + .setNullifier(ByteString.copyFrom(nullifier))), receives); + } + @Test public void transferDescriptionCardinalityIsReportedAsValidationFailure() { GrpcAPI.ShieldedTRC20Parameters.Builder parameters = @@ -58,4 +95,30 @@ public void transferDescriptionCardinalityIsReportedAsValidationFailure() { assertEquals("invalid shielded TRC-20 trigger input: invalid transfer output number", exception.getMessage()); } + + @Test + public void mintRequiresExactlyOneReceiveDescription() throws Exception { + for (int receives : new int[]{0, 2}) { + GrpcAPI.ShieldedTRC20Parameters parameters = withReceives( + GrpcAPI.ShieldedTRC20Parameters.newBuilder().setParameterType("mint"), receives); + ContractValidateException exception = assertThrows(ContractValidateException.class, + () -> triggerInput(parameters, "1", 0)); + assertEquals("invalid shielded TRC-20 trigger input: invalid mint output number", + exception.getMessage()); + } + assertFalse(triggerInput(withReceives( + GrpcAPI.ShieldedTRC20Parameters.newBuilder().setParameterType("mint"), 1), "1", 0) + .getValue().isEmpty()); + } + + @Test + public void burnAcceptsAtMostOneReceiveDescription() throws Exception { + assertFalse(triggerInput(burn(0, "1"), "1", 1).getValue().isEmpty()); + assertFalse(triggerInput(burn(1, "1"), "1", 1).getValue().isEmpty()); + + ContractValidateException exception = assertThrows(ContractValidateException.class, + () -> triggerInput(burn(2, "1"), "1", 1)); + assertEquals("invalid shielded TRC-20 trigger input: invalid burn output number", + exception.getMessage()); + } } diff --git a/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java b/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java index 6681b4c6946..83c82a4bda8 100644 --- a/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java +++ b/framework/src/test/java/org/tron/core/zen/ShieldedTRC20ParametersBuilderTest.java @@ -4,67 +4,97 @@ import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mockStatic; +import com.google.protobuf.ByteString; import java.math.BigInteger; +import java.security.MessageDigest; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.bouncycastle.util.encoders.Hex; import org.junit.Test; import org.mockito.MockedStatic; import org.tron.api.GrpcAPI.BytesMessage; import org.tron.api.GrpcAPI.ShieldedTRC20Parameters; import org.tron.common.utils.ByteUtil; +import org.tron.core.utils.ZenChainParams; import org.tron.protos.contract.ShieldContract; public class ShieldedTRC20ParametersBuilderTest { - private static final String GOLDEN_ONE_TO_ONE = - "00000000000000000000000000000000000000000000000000000000000000c0" - + "0000000000000000000000000000000000000000000000000000000000000220" - + "0000000000000000000000000000000000000000000000000000000000000280" - + "00000000000000000000000000000000000000000000000000000000000003c0" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "000000000000000000000000"; + /** SHA-256 of {@link #encode} per spends-receives-withAsk key; each output is several KB. */ + private static final String[][] BASE_DIGESTS = { + {"1-1-true", + "da4c52cbf2903d98f9eba9c2b537328448d767bee8c495d3c22087e272976238"}, + {"1-1-false", + "b8fa61b5b5230092591d45fa61c65e5bfea2995441cdb373547e378c7670f8df"}, + {"1-2-true", + "77a62c96cf7f03a0ff092661861a53596b9ef57b9f397efcc661482b0ecc1d6b"}, + {"1-2-false", + "b998f3d1fc5e309560476b25e38fdb1f4cb2ec9223ada4734f0c086f0013c29d"}, + {"2-1-true", + "7fb3c1231f29a606aa2509d3d6bc9a0c58d851a8710c21f453d556edcdf41675"}, + {"2-1-false", + "25d93b0be6e8e387d86932b2a2b87f39bea516b543c646d0ce55023250b591ce"}, + {"2-2-true", + "2d4525842ecef92b52c0b3a0dc8689d461e1be08788f29d03a150639f05fe9c6"}, + {"2-2-false", + "aa8cd6ebd1208a53e642d9aa796967d5ebe66a7aca7615965dccd9d727f7b624"}, + }; - private static final String GOLDEN_TWO_TO_TWO = - "00000000000000000000000000000000000000000000000000000000000000c0" - + "0000000000000000000000000000000000000000000000000000000000000360" - + "0000000000000000000000000000000000000000000000000000000000000400" - + "0000000000000000000000000000000000000000000000000000000000000660" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "000000000000000000000000000000000000000000000000"; + private static ByteString fill(int length, int seed) { + byte[] value = new byte[length]; + for (int i = 0; i < length; i++) { + value[i] = (byte) (seed * 31 + i); + } + return ByteString.copyFrom(value); + } - private static final String GOLDEN_ONE_TO_TWO = - "00000000000000000000000000000000000000000000000000000000000000c0" - + "0000000000000000000000000000000000000000000000000000000000000220" - + "0000000000000000000000000000000000000000000000000000000000000280" - + "00000000000000000000000000000000000000000000000000000000000004e0" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "000000000000000000000000000000000000000000000000"; + /** Every field has its protocol size and its own content, so a misplaced field shows. */ + static ShieldedTRC20Parameters populated(int spends, int receives) { + ShieldedTRC20Parameters.Builder parameters = ShieldedTRC20Parameters.newBuilder(); + for (int i = 0; i < spends; i++) { + int seed = 16 * i; + parameters.addSpendDescription(ShieldContract.SpendDescription.newBuilder() + .setNullifier(fill(32, seed + 1)) + .setAnchor(fill(32, seed + 2)) + .setValueCommitment(fill(32, seed + 3)) + .setRk(fill(32, seed + 4)) + .setZkproof(fill(192, seed + 5)) + .setSpendAuthoritySignature(fill(64, seed + 6))); + } + for (int i = 0; i < receives; i++) { + int seed = 16 * i; + parameters.addReceiveDescription(ShieldContract.ReceiveDescription.newBuilder() + .setNoteCommitment(fill(32, seed + 7)) + .setValueCommitment(fill(32, seed + 8)) + .setEpk(fill(32, seed + 9)) + .setZkproof(fill(192, seed + 10)) + .setCEnc(fill(ZenChainParams.ZC_ENCCIPHERTEXT_SIZE, seed + 11)) + .setCOut(fill(ZenChainParams.ZC_OUTCIPHERTEXT_SIZE, seed + 12))); + } + return parameters.setBindingSignature(fill(64, 13)).build(); + } - private static final String GOLDEN_TWO_TO_ONE = - "00000000000000000000000000000000000000000000000000000000000000c0" - + "0000000000000000000000000000000000000000000000000000000000000360" - + "0000000000000000000000000000000000000000000000000000000000000400" - + "0000000000000000000000000000000000000000000000000000000000000540" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "0000000000000000000000000000000000000000000000000000000000000002" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "0000000000000000000000000000000000000000000000000000000000000001" - + "000000000000000000000000"; + /** Differs from the signatures inside the descriptions, so the two paths give distinct bytes. */ + static List requestSignatures(int count) { + List signatures = new ArrayList<>(); + for (int i = 0; i < count; i++) { + signatures.add(BytesMessage.newBuilder().setValue(fill(64, 16 * i + 14)).build()); + } + return signatures; + } + + static String encode(int spends, int receives, boolean withAsk) throws Exception { + return new ShieldedTRC20ParametersBuilder("transfer").getTriggerContractInput( + populated(spends, receives), requestSignatures(spends), BigInteger.ZERO, withAsk, + new byte[21]); + } - private static ShieldedTRC20ParametersBuilder transferBuilder() throws Exception { - return new ShieldedTRC20ParametersBuilder("transfer"); + static String digest(String triggerInput) throws Exception { + return Hex.toHexString(MessageDigest.getInstance("SHA-256").digest(Hex.decode(triggerInput))); } - private static ShieldedTRC20Parameters params(int spends, int receives) { + private static String runEmpty(int spends, int receives) throws Exception { ShieldedTRC20Parameters.Builder parameters = ShieldedTRC20Parameters.newBuilder(); for (int i = 0; i < spends; i++) { parameters.addSpendDescription(ShieldContract.SpendDescription.getDefaultInstance()); @@ -72,17 +102,8 @@ private static ShieldedTRC20Parameters params(int spends, int receives) { for (int i = 0; i < receives; i++) { parameters.addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()); } - return parameters.build(); - } - - private static String run(int spends, int receives) throws Exception { - return run(spends, receives, Collections.emptyList(), true); - } - - private static String run(int spends, int receives, List signatures, - boolean withAsk) throws Exception { - return transferBuilder().getTriggerContractInput( - params(spends, receives), signatures, BigInteger.ZERO, withAsk, new byte[21]); + return new ShieldedTRC20ParametersBuilder("transfer").getTriggerContractInput( + parameters.build(), Collections.emptyList(), BigInteger.ZERO, true, new byte[21]); } @Test @@ -97,23 +118,20 @@ public void invalidCountsAreRejectedBeforeAnyMerge() throws Exception { try (MockedStatic byteUtil = mockStatic(ByteUtil.class)) { for (Object[] counts : invalidCounts) { IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> run((int) counts[0], (int) counts[1])); + () -> runEmpty((int) counts[0], (int) counts[1])); assertEquals(counts[2], exception.getMessage()); } byteUtil.verifyNoInteractions(); } } - /** - * The asymmetric shapes are the ones that pin the arithmetic: both offsets mix the two counts, - * so with spendCount == recvCount a transposition of the two produces identical bytes and the - * symmetric vectors alone would pass under it. - */ @Test - public void validOneAndTwoEntryOutputsRemainByteForByteCompatible() throws Exception { - assertEquals(GOLDEN_ONE_TO_ONE, run(1, 1)); - assertEquals(GOLDEN_TWO_TO_TWO, run(2, 2)); - assertEquals(GOLDEN_ONE_TO_TWO, run(1, 2)); - assertEquals(GOLDEN_TWO_TO_ONE, run(2, 1)); + public void populatedOutputsMatchTheBaseForEveryCountAndSignaturePath() throws Exception { + for (String[] vector : BASE_DIGESTS) { + String[] key = vector[0].split("-"); + String triggerInput = encode(Integer.parseInt(key[0]), Integer.parseInt(key[1]), + Boolean.parseBoolean(key[2])); + assertEquals(vector[0], vector[1], digest(triggerInput)); + } } } From 8734a39f1736a997d6e556f4dd7e071cf2eda309 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Wed, 14 Oct 2026 12:17:45 +0800 Subject: [PATCH 12/14] fix(http): keep Base58 rendering for genesis addresses --- .../services/http/servlets/JsonFormat.java | 4 ++- .../BroadcastHexAddressValidationTest.java | 29 +++++++++++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java b/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java index 07c73847391..fbc381097f6 100644 --- a/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java +++ b/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java @@ -83,6 +83,8 @@ public class JsonFormat { = "Writing to a StringBuilder threw an IOException (should never happen)."; private static final String EXPECTED_STRING = "Expected string."; private static final String MISSING_END_QUOTE = "String missing ending quote."; + // Must stay above the 116-byte genesis witness_address, which is served in Base58Check. + private static final int MAX_BASE58_ADDRESS_SIZE = 128; public static final boolean ALWAYS_OUTPUT_DEFAULT_VALUE_FIELDS = true; public static final Set> MESSAGES = ImmutableSet.of( @@ -863,7 +865,7 @@ static String escapeBytes(ByteString input, final String fieldName, boolean self static String escapeBytesSelfType(ByteString input, final String fieldName) { //Address if (HttpSelfFormatFieldName.isAddressFormat(fieldName)) { - if (input.size() > Constant.TRON_ADDRESS_SIZE) { + if (input.size() > MAX_BASE58_ADDRESS_SIZE) { return ByteArray.toHexString(input.toByteArray()); } return StringUtil.encode58Check(input.toByteArray()); diff --git a/framework/src/test/java/org/tron/core/services/http/servlets/BroadcastHexAddressValidationTest.java b/framework/src/test/java/org/tron/core/services/http/servlets/BroadcastHexAddressValidationTest.java index b4e0c3255d7..e2d44728efe 100644 --- a/framework/src/test/java/org/tron/core/services/http/servlets/BroadcastHexAddressValidationTest.java +++ b/framework/src/test/java/org/tron/core/services/http/servlets/BroadcastHexAddressValidationTest.java @@ -34,7 +34,7 @@ public class BroadcastHexAddressValidationTest { @Test public void rendersOversizedAuthAddressAsHexWithoutBase58Conversion() throws Exception { - for (int size : new int[]{22, 4096, 16384, 65536}) { + for (int size : new int[]{129, 4096, 16384, 65536}) { Wallet wallet = mock(Wallet.class); Transaction transaction = transaction(size); when(wallet.broadcastTransaction(transaction)).thenReturn(Return.newBuilder() @@ -75,12 +75,31 @@ public void jsonFormatterPreservesTwentyByteLogAddresses() { .setAddress(address).build(); JSONObject json = JSONObject.parseObject(JsonFormat.printToString(log, true)); assertEquals(StringUtil.encode58Check(address.toByteArray()), json.getString("address")); - assertEquals(ByteArray.toHexString(bytes(22).toByteArray()), JSONObject.parseObject( - JsonFormat.printToString(log.toBuilder().setAddress(bytes(22)).build(), true)) + assertEquals(StringUtil.encode58Check(bytes(128).toByteArray()), JSONObject.parseObject( + JsonFormat.printToString(log.toBuilder().setAddress(bytes(128)).build(), true)) .getString("address")); - assertEquals(ByteArray.toHexString(bytes(22).toByteArray()), JSONObject.parseObject( - JsonFormat.printToString(log.toBuilder().setAddress(bytes(22)).build(), false)) + assertEquals(ByteArray.toHexString(bytes(129).toByteArray()), JSONObject.parseObject( + JsonFormat.printToString(log.toBuilder().setAddress(bytes(129)).build(), true)) .getString("address")); + assertEquals(ByteArray.toHexString(bytes(129).toByteArray()), JSONObject.parseObject( + JsonFormat.printToString(log.toBuilder().setAddress(bytes(129)).build(), false)) + .getString("address")); + } + + @Test + public void preservesGenesisAddressRendering() { + ByteString witness = ByteString.copyFromUtf8("A new system must allow existing systems to be " + + "linked together without requiring any central control or coordination"); + JSONObject header = JSONObject.parseObject(JsonFormat.printToString( + Protocol.BlockHeader.raw.newBuilder().setWitnessAddress(witness).build(), true)); + assertEquals("A2aRHeJL17i55Nd5FWdtaXJL4RuhcBPnZGSqvDZnw2Qcadhh9hhFBwAkHM52Tuv6d15wMcMgrUc3hi1Hp" + + "CCyAefw5kDgbvbGWjKgJoyER8ZY1w6sy3pxp56YdtiW4LtbCKaWyRpxzXv4mEmh88Y2ipZP8VFpSy8YLqnj", + header.getString("witness_address")); + + ByteString owner = ByteString.copyFromUtf8("0x000000000000000000000"); + JSONObject transfer = JSONObject.parseObject(JsonFormat.printToString( + TransferContract.newBuilder().setOwnerAddress(owner).build(), true)); + assertEquals("7YxAaK71utTpYJ8u4Zna7muWxd1pQwimpGxy8", transfer.getString("owner_address")); } @Test From 437200fc6f83d267e16046bc72d488f5665e9093 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Wed, 14 Oct 2026 13:54:07 +0800 Subject: [PATCH 13/14] refactor(http): simplify genesis address comment --- .../java/org/tron/core/services/http/servlets/JsonFormat.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java b/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java index fbc381097f6..65edb08fd0f 100644 --- a/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java +++ b/framework/src/main/java/org/tron/core/services/http/servlets/JsonFormat.java @@ -83,7 +83,7 @@ public class JsonFormat { = "Writing to a StringBuilder threw an IOException (should never happen)."; private static final String EXPECTED_STRING = "Expected string."; private static final String MISSING_END_QUOTE = "String missing ending quote."; - // Must stay above the 116-byte genesis witness_address, which is served in Base58Check. + // Keeps the genesis block addresses compatible with their Base58Check rendering. private static final int MAX_BASE58_ADDRESS_SIZE = 128; public static final boolean ALWAYS_OUTPUT_DEFAULT_VALUE_FIELDS = true; From 75939ea45d75c417e8789068256c16cd2c67c07e Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Wed, 14 Oct 2026 14:54:14 +0800 Subject: [PATCH 14/14] refactor(shielded): spell out transfer count checks --- .../org/tron/core/zen/ShieldedTRC20ParametersBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java index ea91ea6b10f..a3e44187b2c 100644 --- a/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java +++ b/framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java @@ -429,10 +429,10 @@ private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transfe List recvDescs = transferParams.getReceiveDescriptionList(); long spendCount = spendDescs.size(); long recvCount = recvDescs.size(); - if (spendCount < 1 || spendCount > 2) { + if (spendCount != 1 && spendCount != 2) { throw new IllegalArgumentException("invalid transfer input number"); } - if (recvCount < 1 || recvCount > 2) { + if (recvCount != 1 && recvCount != 2) { throw new IllegalArgumentException("invalid transfer output number"); }