fix(api): harden external input validation

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);
   }
 
@@ -4325,7 +4337,12 @@ public BytesMessage getTriggerInputForShieldedTRC20Contract(
 
     ShieldedTRC20Parameters shieldedTRC20Parameters = request.getShieldedTRC20Parameters();
     List<BytesMessage> 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];
@@ -4390,9 +4407,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");
     }

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 (IllegalArgumentException e) {
+      Util.writeError(response, e.getMessage());
+      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) {
-      try {
-        response.getWriter()
-            .println("{\"Error\": " + "\"INVALID address, " + e.getMessage() + "\"}");
-      } catch (IOException ioe) {
-        logger.debug("IOException: {}", ioe.getMessage());
-      }
     } catch (Exception e) {
       Util.processError(e, response);
     }

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,30 +16,25 @@ 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.writeError(response, e.getMessage());
+      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) {
-      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);
     }
   }
 

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,12 @@ 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 int BODY_READ_BUFFER_SIZE = 4096;
+  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";
@@ -431,14 +439,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) {
@@ -505,6 +547,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");
@@ -534,6 +579,16 @@ public static void processError(Exception e, HttpServletResponse response) {
     }
   }
 
+  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);
@@ -559,16 +614,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);
+    }
+    if (StringUtils.isBlank(addressStr)) {
+      throw new IllegalArgumentException(INVALID_ADDRESS);
+    }
+
+    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;
   }
@@ -602,14 +684,24 @@ private static String checkGetParam(HttpServletRequest request, String key) thro
     return null;
   }
 
-  public static String getRequestValue(HttpServletRequest request) throws IOException {
-    BufferedReader reader = new BufferedReader(new InputStreamReader(request.getInputStream()));
-    String line;
+  /**
+   * Reads the request body. This is the only body-reading path in the http layer that does not go
+   * through {@link PostParams}, so the {@link #checkBodySize} every other path performs is applied
+   * here as well.
+   */
+  public static String getRequestValue(HttpServletRequest request) throws Exception {
     StringBuilder sb = new StringBuilder();
-    while ((line = reader.readLine()) != null) {
-      sb.append(line);
+    char[] buffer = new char[BODY_READ_BUFFER_SIZE];
+    try (BufferedReader reader = new BufferedReader(
+        new InputStreamReader(request.getInputStream()))) {
+      int read;
+      while ((read = reader.read(buffer)) != -1) {
+        sb.append(buffer, 0, read);
+      }
     }
-    return sb.toString();
+    String value = sb.toString();
+    checkBodySize(value);
+    return value;
   }
 
   public static List<Log> convertLogAddressToTronAddress(TransactionInfo transactionInfo) {