diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index ac54cb2b7ff..c7b6a553bd0 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"; @@ -3201,6 +3204,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( @@ -3230,6 +3236,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( @@ -4214,6 +4223,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); } 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 1fbd94fe690..cb9fff6ccf9 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 @@ -1,10 +1,8 @@ package org.tron.core.services.http; -import java.io.IOException; 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; @@ -18,21 +16,20 @@ public class GetBrokerageServlet extends RateLimiterServlet { private Manager manager; protected void doGet(HttpServletRequest request, HttpServletResponse response) { + byte[] address; + try { + address = Util.getAddress(request); + } catch (Exception e) { + Util.processAddressParamError(e, response); + return; + } try { int value = 0; - byte[] address = Util.getAddress(request); - long cycle = manager.getDynamicPropertiesStore().getCurrentCycleNumber(); if (address != null) { + long cycle = manager.getDynamicPropertiesStore().getCurrentCycleNumber(); value = manager.getDelegationStore().getBrokerage(cycle, address); } response.getWriter().println("{\"brokerage\": " + value + "}"); - } catch (DecoderException | IllegalArgumentException e) { - try { - response.getWriter() - .println("{\"Error\": " + "\"INVALID address, " + e.getMessage() + "\"}"); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } } catch (Exception e) { Util.processError(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 61b88d1160f..18ad51de11f 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 @@ -1,10 +1,8 @@ package org.tron.core.services.http; -import java.io.IOException; 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; @@ -18,9 +16,15 @@ public class GetRewardServlet extends RateLimiterServlet { private Manager manager; protected void doGet(HttpServletRequest request, HttpServletResponse response) { + byte[] address; + try { + address = Util.getAddress(request); + } catch (Exception e) { + Util.processAddressParamError(e, response); + return; + } try { long value = 0; - byte[] address = Util.getAddress(request); if (address != null) { value = manager.getMortgageService().queryReward(address); } @@ -28,20 +32,8 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) { ? "{\"reward\": \"" + value + "\"}" : "{\"reward\": " + value + "}"; response.getWriter().println(out); - } catch (DecoderException | IllegalArgumentException e) { - try { - response.getWriter() - .println("{\"Error\": " + "\"INVALID address, " + e.getMessage() + "\"}"); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } } catch (Exception e) { - logger.error("", e); - try { - response.getWriter().println(Util.printErrorMsg(e)); - } catch (IOException ioe) { - logger.debug("IOException: {}", ioe.getMessage()); - } + Util.processError(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 5be2495e1f7..e1c4992aa67 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; @@ -41,6 +42,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; @@ -69,6 +71,11 @@ 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; + private static final String INVALID_ADDRESS = "Invalid address"; + 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"; @@ -433,12 +440,27 @@ public static String getHexString(final String string) { 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); + if (rawValue instanceof String + && ((String) rawValue).length() > MAX_JSON_INTEGER_VALUE_LENGTH) { + throw new InvalidParameterException(INVALID_PERMISSION_ID); + } + BigDecimal value = jsonObject.getBigDecimal(PERMISSION_ID); + if (value == null) { + throw new InvalidParameterException(INVALID_PERMISSION_ID); + } + // Check exactness first, then retain getInteger's legacy string syntax. + value.intValueExact(); + permissionId = jsonObject.getInteger(PERMISSION_ID); + } catch (NumberFormatException | ArithmeticException | JSONException e) { + throw new InvalidParameterException(INVALID_PERMISSION_ID); + } + return setTransactionPermissionId(permissionId, transaction); } public static Transaction setTransactionPermissionId(int permissionId, Transaction transaction) { @@ -505,6 +527,12 @@ 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); + if (rawValue instanceof String + && ((String) rawValue).length() > MAX_JSON_INTEGER_VALUE_LENGTH) { + throw new InvalidParameterException( + "key [" + key + "] exceeds " + MAX_JSON_INTEGER_VALUE_LENGTH + " characters"); + } BigDecimal bigDecimal = jsonObject.getBigDecimal(key); if (required && bigDecimal == null) { throw new InvalidParameterException("key [" + key + "] does not exist"); @@ -534,6 +562,35 @@ public static void processError(Exception e, HttpServletResponse response) { } } + /** + * Reports a failure to read the address out of the request, shared by the address-keyed + * endpoints and their solidity/PBFT mirrors so that all of them answer a given malformed + * request identically. Apply it only to exceptions raised by {@link #getAddress}: a + * failure from the service layer must not reach the caller labelled as a bad address. + * A malformed json body reports a fixed message, because the parser quotes the offending + * token and that token is caller input. + */ + public static void processAddressParamError(Exception e, HttpServletResponse response) { + if (e instanceof JSONException) { + logger.debug("malformed json body: {}", e.getMessage()); + writeError(response, INVALID_JSON_BODY); + } else if (e instanceof IllegalArgumentException) { + writeError(response, "INVALID address, " + e.getMessage()); + } else { + processError(e, response); + } + } + + private static void writeError(HttpServletResponse response, String message) { + JSONObject error = new JSONObject(); + error.put("Error", message); + try { + response.getWriter().println(error.toJSONString()); + } catch (IOException ioe) { + logger.debug("IOException: {}", ioe.getMessage()); + } + } + public static String convertOutput(Account account) { if (account.getAssetIssuedID().isEmpty()) { return JsonFormat.printToString(account, false); @@ -560,15 +617,29 @@ public static void printAccount(Account reply, HttpServletResponse response, Boo } 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); - } + // an absent or blank address keeps the legacy contract: the caller decides the default + if (StringUtils.isBlank(addressStr)) { + return null; + } + + 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); + } + + 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); + } + // base58 is validated inside the decoder; hex used to be returned unchecked + if (address == null || (hex && !DecodeUtil.addressValid(address))) { + throw new IllegalArgumentException(INVALID_ADDRESS); } return 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 4ee4f75a171..147eabcc3bd 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,19 @@ 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 || recvCount < 1 || recvCount > 2) { + throw new IllegalArgumentException("invalid transfer description 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 +451,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 +464,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 +479,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/WalletGetBigIntegerFromStringDoSTest.java b/framework/src/test/java/org/tron/core/WalletGetBigIntegerFromStringDoSTest.java new file mode 100644 index 00000000000..16674a68ad3 --- /dev/null +++ b/framework/src/test/java/org/tron/core/WalletGetBigIntegerFromStringDoSTest.java @@ -0,0 +1,105 @@ +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.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 WalletGetBigIntegerFromStringDoSTest { + + private static Method bigIntegerFromString() throws Exception { + Method method = Wallet.class.getDeclaredMethod("getBigIntegerFromString", String.class); + method.setAccessible(true); + return method; + } + + private static BigInteger parse(String value) throws Exception { + try { + return (BigInteger) bigIntegerFromString().invoke(new Wallet(), value); + } catch (InvocationTargetException exception) { + throw (Exception) exception.getCause(); + } + } + + private static void checkRange(BigInteger value) throws Exception { + Method method = Wallet.class.getDeclaredMethod("checkBigIntegerRange", BigInteger.class); + method.setAccessible(true); + try { + method.invoke(new Wallet(), value); + } catch (InvocationTargetException exception) { + throw (Exception) exception.getCause(); + } + } + + private static String repeat(char character, int count) { + StringBuilder builder = new StringBuilder(count); + for (int i = 0; i < count; i++) { + builder.append(character); + } + return builder.toString(); + } + + @Test + public void amountLengthBoundaryIsAppliedAfterTrimAndBeforeParsing() throws Exception { + assertEquals(BigInteger.ZERO, parse(" ")); + assertEquals(BigInteger.ZERO, parse(" " + repeat('0', 80) + " ")); + + String oversized = 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(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 ")); + } + + @Test + public void disabledFeatureGateRunsBeforeAmountOrDescriptionValidation() { + Wallet wallet = new Wallet(); + GrpcAPI.ShieldedTRC20TriggerContractParameters request = + GrpcAPI.ShieldedTRC20TriggerContractParameters.newBuilder() + .setAmount(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/WalletGetContractDoSTest.java b/framework/src/test/java/org/tron/core/WalletGetContractDoSTest.java new file mode 100644 index 00000000000..40e86427afd --- /dev/null +++ b/framework/src/test/java/org/tron/core/WalletGetContractDoSTest.java @@ -0,0 +1,122 @@ +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.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 WalletGetContractDoSTest { + + private static class Fixture { + private final Wallet wallet; + private final AccountStore accountStore; + + private Fixture(Wallet wallet, AccountStore accountStore) { + this.wallet = wallet; + this.accountStore = accountStore; + } + } + + private static Fixture walletWithMissingAccount() 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); + return new Fixture(wallet, 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) { + Fixture fixture = walletWithMissingAccount(); + try (MockedStatic base58 = mockStatic(Base58.class)) { + assertNull(fixture.wallet.getContract(bytes(invalidAddress))); + assertNull(fixture.wallet.getContractInfo(bytes(invalidAddress))); + verifyNoInteractions(fixture.accountStore); + base58.verifyNoInteractions(); + } + } + } + + @Test + public void canonicalMissingAddressRetainsNoResultBehavior() throws Exception { + Fixture fixture = walletWithMissingAccount(); + byte[] address = canonicalAddress(); + + assertNull(fixture.wallet.getContract(bytes(address))); + assertNull(fixture.wallet.getContractInfo(bytes(address))); + + verify(fixture.accountStore, times(2)).get(aryEq(address)); + } + + @Test + public void canonicalExistingAddressRetainsContractResult() throws Exception { + Wallet wallet = new Wallet(); + 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); + + Field field = Wallet.class.getDeclaredField("chainBaseManager"); + field.setAccessible(true); + field.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..2e1e81cd3ab 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -907,6 +907,7 @@ public void testGetTriggerInputForShieldedTRC20Contract1() GrpcAPI.ShieldedTRC20Parameters shieldedTRC20Parameters = GrpcAPI.ShieldedTRC20Parameters.newBuilder() .addSpendDescription(spendDescription) + .addReceiveDescription(ShieldContract.ReceiveDescription.getDefaultInstance()) .setParameterType("transfer") .build(); GrpcAPI.BytesMessage bytesMessage = @@ -1399,8 +1400,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 +1422,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); 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..48437715f1a 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,6 +1,8 @@ package org.tron.core.services.http; -import java.io.UnsupportedEncodingException; +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.util.Arrays; import javax.annotation.Resource; import org.junit.Assert; import org.junit.Test; @@ -8,13 +10,15 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.utils.StringUtil; +import org.tron.core.Wallet; import org.tron.core.config.args.Args; import org.tron.json.JSONObject; public class GetBrokerageServletTest extends BaseTest { @Resource - private GetBrokerageServlet getBrokerageServlet; + private GetBrokerageServlet getBrokerageServlet; static { Args.setParam( @@ -24,84 +28,176 @@ public class GetBrokerageServletTest extends BaseTest { ); } - public MockHttpServletRequest createRequest(String contentType) { + private static MockHttpServletRequest postRequest(String contentType) { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("POST"); request.setContentType(contentType); - request.setCharacterEncoding("UTF-8"); + request.setCharacterEncoding(UTF_8.name()); return request; } - @Test - public void getBrokerageValueByJsonTest() { - int expect = 20; - String jsonParam = "{\"address\": \"TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W\"}"; - MockHttpServletRequest request = createRequest("application/json"); - request.setContent(jsonParam.getBytes()); + private static MockHttpServletRequest getRequest(String address) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + private static MockHttpServletRequest formRequest(String address) { + MockHttpServletRequest request = postRequest("application/x-www-form-urlencoded"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + private static MockHttpServletRequest jsonRequest(String address) { + return jsonRequest(address, "application/json"); + } + + private static MockHttpServletRequest jsonRequest(String address, String contentType) { + MockHttpServletRequest request = postRequest(contentType); + String json = address == null ? "{}" : "{\"address\":\"" + address + "\"}"; + request.setContent(json.getBytes(UTF_8)); + return request; + } + + private static String wrongPrefixAddress() { + byte[] address = new byte[21]; + address[0] = (byte) (Wallet.getAddressPreFixByte() + 1); + return StringUtil.encode58Check(address); + } + + private static void assertInvalid(MockHttpServletRequest request, String submitted) + throws Exception { 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()); + GetBrokerageServlet servlet = new GetBrokerageServlet(); + if ("GET".equals(request.getMethod())) { + servlet.doGet(request, response); + } else { + servlet.doPost(request, response); + } + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals("INVALID address, Invalid address", result.get("Error")); + Assert.assertNull(result.get("brokerage")); + if (submitted != null && !submitted.isEmpty()) { + Assert.assertFalse(response.getContentAsString().contains(submitted)); } } + private static String canonicalNoStateAddress() { + byte[] address = new byte[21]; + Arrays.fill(address, (byte) 8); + address[0] = Wallet.getAddressPreFixByte(); + return StringUtil.encode58Check(address); + } @Test - public void getBrokerageByJsonUTF8Test() { - int expect = 20; - String jsonParam = "{\"address\": \"TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W\"}"; - MockHttpServletRequest request = createRequest("application/json; charset=utf-8"); - request.setContent(jsonParam.getBytes()); + public void getBrokerageValueByJsonTest() throws Exception { + MockHttpServletRequest request = jsonRequest("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()); - } + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(20, (int) result.get("brokerage")); } @Test - public void getBrokerageValueTest() { - int expect = 20; - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - request.addParameter("address", "TGSzEq4t7oMTRcn1VxDghRu5r5bWAE5D1W"); + public void getBrokerageValueTest() throws Exception { + MockHttpServletRequest request = formRequest("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()); + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(20, (int) result.get("brokerage")); + } + + @Test + public void validAddressWithoutStateRetainsServiceDefault() throws Exception { + MockHttpServletRequest request = getRequest(canonicalNoStateAddress()); + MockHttpServletResponse response = new MockHttpServletResponse(); + getBrokerageServlet.doGet(request, response); + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(20, (int) result.get("brokerage")); + 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"; + // a base58check payload whose first byte is not the address prefix; unlike a raw + // "42..." string this reaches the prefix check instead of failing the alphabet + String wrongPrefix = wrongPrefixAddress(); + StringBuilder oversizedHex = new StringBuilder("41"); + for (int i = 0; i < 8192; i++) { + oversizedHex.append('0'); } + + assertInvalid(getRequest(invalidBase58), invalidBase58); + assertInvalid(getRequest(illegalBase58Char), illegalBase58Char); + assertInvalid(getRequest(wrongPrefix), wrongPrefix); + assertInvalid(getRequest(oversizedHex.toString()), oversizedHex.toString()); + assertInvalid(jsonRequest(invalidBase58), invalidBase58); + assertInvalid(jsonRequest(invalidHex), invalidHex); } + /** + * A missing or blank address keeps answering with the service default, as it did before + * address validation was tightened; only a present-but-malformed address is rejected. + */ @Test - public void getByBlankParamTest() { - int expect = 0; - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); - request.addParameter("address", ""); + public void missingOrBlankAddressRetainsServiceDefault() throws Exception { + assertServiceDefault(getRequest(null)); + assertServiceDefault(getRequest("")); + assertServiceDefault(formRequest(null)); + assertServiceDefault(formRequest("")); + assertServiceDefault(jsonRequest(null)); + } + + private void assertServiceDefault(MockHttpServletRequest request) throws Exception { 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()); + if ("GET".equals(request.getMethod())) { + getBrokerageServlet.doGet(request, response); + } else { + getBrokerageServlet.doPost(request, response); } + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(0, (int) result.get("brokerage")); + Assert.assertNull(result.get("Error")); + } + + /** + * 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 malformed address: on the form branch no address would be found at all and + * the service default would come back instead of the rejection. + */ + @Test + public void jsonBodyWithCharsetSuffixIsParsed() throws Exception { + String invalidBase58 = "Taaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + assertInvalid(jsonRequest(invalidBase58, "application/json; charset=utf-8"), invalidBase58); + } + + @Test + public void malformedJsonBodyIsRejectedWithoutEchoingIt() throws Exception { + String marker = "UniqueMarkerValue"; + MockHttpServletRequest request = postRequest("application/json"); + request.setContent(("{\"address\":" + marker + "}").getBytes(UTF_8)); + MockHttpServletResponse response = new MockHttpServletResponse(); + new GetBrokerageServlet().doPost(request, response); + + String body = response.getContentAsString(); + Assert.assertFalse(body.contains(marker)); + JSONObject result = JSONObject.parseObject(body); + Assert.assertEquals("INVALID JSON body", result.get("Error")); + Assert.assertNull(result.get("brokerage")); } } 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/GetRewardServletTest.java b/framework/src/test/java/org/tron/core/services/http/GetRewardServletTest.java index 9afa5607a66..8db19e6085e 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 @@ -1,8 +1,9 @@ package org.tron.core.services.http; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.tron.common.utils.Commons.decodeFromBase58Check; -import java.io.UnsupportedEncodingException; +import java.util.Arrays; import javax.annotation.Resource; import lombok.extern.slf4j.Slf4j; import org.junit.Assert; @@ -12,6 +13,8 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.utils.StringUtil; +import org.tron.core.Wallet; import org.tron.core.config.args.Args; import org.tron.core.db.Manager; import org.tron.core.service.MortgageService; @@ -31,24 +34,79 @@ 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) { + private static MockHttpServletRequest postRequest(String contentType) { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("POST"); request.setContentType(contentType); - request.setCharacterEncoding("UTF-8"); + request.setCharacterEncoding(UTF_8.name()); return request; } + private static MockHttpServletRequest getRequest(String address) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + private static MockHttpServletRequest formRequest(String address) { + MockHttpServletRequest request = postRequest("application/x-www-form-urlencoded"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + private static MockHttpServletRequest jsonRequest(String address) { + return jsonRequest(address, "application/json"); + } + + private static MockHttpServletRequest jsonRequest(String address, String contentType) { + MockHttpServletRequest request = postRequest(contentType); + String json = address == null ? "{}" : "{\"address\":\"" + address + "\"}"; + request.setContent(json.getBytes(UTF_8)); + return request; + } + + private static String wrongPrefixAddress() { + byte[] address = new byte[21]; + address[0] = (byte) (Wallet.getAddressPreFixByte() + 1); + return StringUtil.encode58Check(address); + } + + private static void assertInvalid(MockHttpServletRequest request, String submitted) + throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + GetRewardServlet servlet = new GetRewardServlet(); + if ("GET".equals(request.getMethod())) { + servlet.doGet(request, response); + } else { + servlet.doPost(request, response); + } + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals("INVALID address, Invalid address", result.get("Error")); + Assert.assertNull(result.get("reward")); + if (submitted != null && !submitted.isEmpty()) { + Assert.assertFalse(response.getContentAsString().contains(submitted)); + } + } + + private static String canonicalNoStateAddress() { + byte[] address = new byte[21]; + Arrays.fill(address, (byte) 7); + address[0] = Wallet.getAddressPreFixByte(); + return StringUtil.encode58Check(address); + } + @Before public void init() { manager.getDynamicPropertiesStore().saveChangeDelegation(1); @@ -58,107 +116,109 @@ public void init() { } @Test - public void getRewardValueByJsonTest() { - int expect = 138181; - String jsonParam = "{\"address\": \"TNboetpFgv9SqMoHvaVt626NLXETnbdW1K\"}"; - MockHttpServletRequest request = createRequest("application/json"); + public void getRewardValueByJsonTest() throws Exception { + MockHttpServletRequest request = jsonRequest("TNboetpFgv9SqMoHvaVt626NLXETnbdW1K"); 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()); - } + getRewardServlet.doPost(request, response); + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(138181, (int) result.get("reward")); } @Test - public void getRewardByJsonUTF8Test() { - int expect = 138181; - String jsonParam = "{\"address\": \"TNboetpFgv9SqMoHvaVt626NLXETnbdW1K\"}"; - MockHttpServletRequest request = createRequest("application/json; charset=utf-8"); + public void getRewardValueTest() throws Exception { + mortgageService.payStandbyWitness(); + MockHttpServletRequest request = formRequest("TNboetpFgv9SqMoHvaVt626NLXETnbdW1K"); 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()); - } + getRewardServlet.doPost(request, response); + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(138181, (int) result.get("reward")); } @Test - public void getRewardValueTest() { - int expect = 138181; - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); + public void validAddressWithoutStateRetainsServiceDefault() throws Exception { + MockHttpServletRequest request = getRequest(canonicalNoStateAddress()); MockHttpServletResponse response = new MockHttpServletResponse(); - 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()); - } + getRewardServlet.doGet(request, response); + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(0, (int) result.get("reward")); + Assert.assertNull(result.get("Error")); } @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()); + 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"; + // a base58check payload whose first byte is not the address prefix; unlike a raw + // "42..." string this reaches the prefix check instead of failing the alphabet + String wrongPrefix = wrongPrefixAddress(); + StringBuilder oversizedHex = new StringBuilder("41"); + for (int i = 0; i < 8192; i++) { + oversizedHex.append('0'); } + + assertInvalid(getRequest(invalidBase58), invalidBase58); + assertInvalid(getRequest(illegalBase58Char), illegalBase58Char); + assertInvalid(getRequest(wrongPrefix), wrongPrefix); + assertInvalid(getRequest(oversizedHex.toString()), oversizedHex.toString()); + assertInvalid(jsonRequest(invalidBase58), invalidBase58); + assertInvalid(jsonRequest(invalidHex), invalidHex); } + /** + * A missing or blank address keeps answering with the service default, as it did before + * address validation was tightened; only a present-but-malformed address is rejected. + */ @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"); + public void missingOrBlankAddressRetainsServiceDefault() throws Exception { + assertServiceDefault(getRequest(null)); + assertServiceDefault(getRequest("")); + assertServiceDefault(formRequest(null)); + assertServiceDefault(formRequest("")); + assertServiceDefault(jsonRequest(null)); + } + + private void assertServiceDefault(MockHttpServletRequest request) throws Exception { 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()); + if ("GET".equals(request.getMethod())) { + getRewardServlet.doGet(request, response); + } else { + getRewardServlet.doPost(request, response); } + + JSONObject result = JSONObject.parseObject(response.getContentAsString()); + Assert.assertEquals(0, (int) result.get("reward")); + Assert.assertNull(result.get("Error")); } + /** + * 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 malformed address: on the form branch no address would be found at all and + * the service default would come back instead of the rejection. + */ @Test - public void getRewardByOversizedIllegalCharAddressTest() { - MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded"); + public void jsonBodyWithCharsetSuffixIsParsed() throws Exception { + String invalidBase58 = "Taaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + assertInvalid(jsonRequest(invalidBase58, "application/json; charset=utf-8"), invalidBase58); + } + + @Test + public void malformedJsonBodyIsRejectedWithoutEchoingIt() throws Exception { + String marker = "UniqueMarkerValue"; + MockHttpServletRequest request = postRequest("application/json"); + request.setContent(("{\"address\":" + marker + "}").getBytes(UTF_8)); 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()); - } - } + String body = response.getContentAsString(); + Assert.assertFalse(body.contains(marker)); + JSONObject result = JSONObject.parseObject(body); + Assert.assertEquals("INVALID JSON body", result.get("Error")); + Assert.assertNull(result.get("reward")); + } } diff --git a/framework/src/test/java/org/tron/core/services/http/JsonLongValueBigDecimalDoSTest.java b/framework/src/test/java/org/tron/core/services/http/JsonLongValueBigDecimalDoSTest.java new file mode 100644 index 00000000000..493f79814e4 --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/JsonLongValueBigDecimalDoSTest.java @@ -0,0 +1,61 @@ +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.junit.Test; +import org.tron.json.JSONObject; + +public class JsonLongValueBigDecimalDoSTest { + + private static JSONObject value(String value) { + return JSONObject.parseObject("{\"id\":\"" + value + "\"}"); + } + + private static String repeat(char character, int count) { + StringBuilder builder = new StringBuilder(count); + for (int i = 0; i < count; i++) { + builder.append(character); + } + return builder.toString(); + } + + @Test + public void stringLengthBoundaryIsEnforcedBeforeDecimalConversion() { + assertEquals(0L, Util.getJsonLongValue(value(repeat('0', 64)), "id", true)); + + String oversized = repeat('9', 65); + InvalidParameterException exception = assertThrows(InvalidParameterException.class, + () -> Util.getJsonLongValue(value(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)); + } + + @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..f30b99d6e12 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,6 +1,9 @@ package org.tron.core.services.http; import com.google.protobuf.ByteString; +import java.math.BigDecimal; +import java.security.InvalidParameterException; +import java.util.Arrays; import javax.annotation.Resource; import org.junit.Assert; import org.junit.Before; @@ -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,107 @@ public void testPrintSignWeightTooManySigsHttpPath() { Assert.assertTrue(jsonObject.getJSONObject("result").getString("message") .contains("too many signatures")); } + + 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) + "\""))); + } + + @Test + public void testPermissionIdRejectsOverlongStringBeforeBigDecimalConversion() { + JSONObject jsonObject = new JSONObject() { + @Override + public BigDecimal getBigDecimal(String key) { + throw new AssertionError("overlong Permission_id must be rejected before conversion"); + } + }; + char[] digits = new char[65]; + Arrays.fill(digits, '0'); + digits[digits.length - 1] = '2'; + jsonObject.put(Util.PERMISSION_ID, new String(digits)); + + InvalidParameterException e = Assert.assertThrows(InvalidParameterException.class, + () -> Util.setTransactionPermissionId(jsonObject, buildTransferTransaction())); + Assert.assertTrue(e.getMessage().contains(Util.PERMISSION_ID)); + } + + 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"))); + } } diff --git a/framework/src/test/java/org/tron/core/services/interfaceOnPBFT/http/RewardBrokerageAddressValidationTest.java b/framework/src/test/java/org/tron/core/services/interfaceOnPBFT/http/RewardBrokerageAddressValidationTest.java new file mode 100644 index 00000000000..65a2857bead --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/interfaceOnPBFT/http/RewardBrokerageAddressValidationTest.java @@ -0,0 +1,57 @@ +package org.tron.core.services.interfaceOnPBFT.http; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; + +import java.lang.reflect.Field; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.core.db.Manager; +import org.tron.core.services.WalletOnCursor; +import org.tron.core.services.interfaceOnPBFT.WalletOnPBFT; +import org.tron.json.JSONObject; + +public class RewardBrokerageAddressValidationTest { + + private static WalletOnPBFT walletOnPbft() throws Exception { + WalletOnPBFT wallet = new WalletOnPBFT(); + Field manager = WalletOnCursor.class.getDeclaredField("dbManager"); + manager.setAccessible(true); + manager.set(wallet, mock(Manager.class)); + return wallet; + } + + private static MockHttpServletRequest request(String method, String address) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod(method); + request.setContentType("application/x-www-form-urlencoded"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + private static void inject(Object servlet, String fieldName, Object value) throws Exception { + Field field = servlet.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(servlet, value); + } + + @Test + public void rewardAndBrokerageMirrorsRejectMalformedAddresses() throws Exception { + GetRewardOnPBFTServlet reward = new GetRewardOnPBFTServlet(); + inject(reward, "walletOnPBFT", walletOnPbft()); + MockHttpServletResponse rewardResponse = new MockHttpServletResponse(); + reward.doGet(request("GET", "4100"), rewardResponse); + assertEquals("INVALID address, Invalid address", + JSONObject.parseObject(rewardResponse.getContentAsString()).get("Error")); + + GetBrokerageOnPBFTServlet brokerage = new GetBrokerageOnPBFTServlet(); + inject(brokerage, "walletOnPBFT", walletOnPbft()); + MockHttpServletResponse brokerageResponse = new MockHttpServletResponse(); + brokerage.doPost(request("POST", "4100"), brokerageResponse); + assertEquals("INVALID address, Invalid address", + JSONObject.parseObject(brokerageResponse.getContentAsString()).get("Error")); + } +} diff --git a/framework/src/test/java/org/tron/core/services/interfaceOnSolidity/http/RewardBrokerageAddressValidationTest.java b/framework/src/test/java/org/tron/core/services/interfaceOnSolidity/http/RewardBrokerageAddressValidationTest.java new file mode 100644 index 00000000000..6df2ccccf5d --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/interfaceOnSolidity/http/RewardBrokerageAddressValidationTest.java @@ -0,0 +1,57 @@ +package org.tron.core.services.interfaceOnSolidity.http; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; + +import java.lang.reflect.Field; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.core.db.Manager; +import org.tron.core.services.WalletOnCursor; +import org.tron.core.services.interfaceOnSolidity.WalletOnSolidity; +import org.tron.json.JSONObject; + +public class RewardBrokerageAddressValidationTest { + + private static WalletOnSolidity walletOnSolidity() throws Exception { + WalletOnSolidity wallet = new WalletOnSolidity(); + Field manager = WalletOnCursor.class.getDeclaredField("dbManager"); + manager.setAccessible(true); + manager.set(wallet, mock(Manager.class)); + return wallet; + } + + private static MockHttpServletRequest request(String method, String address) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod(method); + request.setContentType("application/x-www-form-urlencoded"); + if (address != null) { + request.addParameter("address", address); + } + return request; + } + + private static void inject(Object servlet, String fieldName, Object value) throws Exception { + Field field = servlet.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(servlet, value); + } + + @Test + public void rewardAndBrokerageMirrorsRejectMalformedAddresses() throws Exception { + GetRewardOnSolidityServlet reward = new GetRewardOnSolidityServlet(); + inject(reward, "walletOnSolidity", walletOnSolidity()); + MockHttpServletResponse rewardResponse = new MockHttpServletResponse(); + reward.doGet(request("GET", "4100"), rewardResponse); + assertEquals("INVALID address, Invalid address", + JSONObject.parseObject(rewardResponse.getContentAsString()).get("Error")); + + GetBrokerageOnSolidityServlet brokerage = new GetBrokerageOnSolidityServlet(); + inject(brokerage, "walletOnSolidity", walletOnSolidity()); + MockHttpServletResponse brokerageResponse = new MockHttpServletResponse(); + brokerage.doPost(request("POST", "4100"), brokerageResponse); + assertEquals("INVALID address, Invalid address", + JSONObject.parseObject(brokerageResponse.getContentAsString()).get("Error")); + } +} diff --git a/framework/src/test/java/org/tron/core/zen/ShieldedTRC20MergeDoSTest.java b/framework/src/test/java/org/tron/core/zen/ShieldedTRC20MergeDoSTest.java new file mode 100644 index 00000000000..71b5986965a --- /dev/null +++ b/framework/src/test/java/org/tron/core/zen/ShieldedTRC20MergeDoSTest.java @@ -0,0 +1,78 @@ +package org.tron.core.zen; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mockStatic; + +import java.math.BigInteger; +import java.util.Collections; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.tron.api.GrpcAPI.ShieldedTRC20Parameters; +import org.tron.common.utils.ByteUtil; +import org.tron.protos.contract.ShieldContract; + +public class ShieldedTRC20MergeDoSTest { + + 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 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 transferBuilder().getTriggerContractInput( + params(spends, receives), Collections.emptyList(), BigInteger.ZERO, true, new byte[21]); + } + + @Test + public void invalidCountsAreRejectedBeforeAnyMerge() throws Exception { + int[][] invalidCounts = {{0, 1}, {3, 1}, {1, 0}, {1, 3}, {1, 10_000}}; + try (MockedStatic byteUtil = mockStatic(ByteUtil.class)) { + for (int[] counts : invalidCounts) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> run(counts[0], counts[1])); + assertTrue(exception.getMessage().contains("invalid transfer description number")); + } + byteUtil.verifyNoInteractions(); + } + } + + @Test + public void validOneAndTwoEntryOutputsRemainByteForByteCompatible() throws Exception { + assertEquals(GOLDEN_ONE_TO_ONE, run(1, 1)); + assertEquals(GOLDEN_TWO_TO_TWO, run(2, 2)); + } +}