Skip to content

fix(api): harden external input validation #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
0xbigapple wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

fix(api): harden external input validation #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fe4d7e
fix(api): harden external input validation
0xbigapple Aug 19, 2026
dd868b9
fix(api): reject only malformed addresses, keep blank-address default
0xbigapple Aug 20, 2026
14d069f
fix(shielded): require exactly one receive description for mint
0xbigapple Aug 20, 2026
d2e1cdb
refactor: name the shielded amount cap
0xbigapple Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions framework/src/main/java/org/tron/core/Wallet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}

Expand Down
19 changes: 8 additions & 11 deletions framework/src/main/java/org/tron/core/services/http/GetBrokerageServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
}
Expand Down
24 changes: 8 additions & 16 deletions framework/src/main/java/org/tron/core/services/http/GetRewardServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -18,30 +16,24 @@ 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);
}
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);
}
}

Expand Down
95 changes: 83 additions & 12 deletions framework/src/main/java/org/tron/core/services/http/Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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

@cubic-dev-ai cubic-dev-ai Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new 64-character limit guards only string-form values (rawValue instanceof String). A bare numeric JSON literal with many digits is parsed by Jackson into a numeric node, bypasses the guard, and still undergoes full BigDecimal/BigInteger conversion before the exactness check rejects it. If the stated intent is to bound conversion cost before BigDecimal, extend the check to non-string numeric primitives (e.g. reject oversized Number values before getBigDecimal), and confirm the Jackson number-length limit actually caps this path.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/services/http/Util.java, line 446:

<comment>The new 64-character limit guards only string-form values (`rawValue instanceof String`). A bare numeric JSON literal with many digits is parsed by Jackson into a numeric node, bypasses the guard, and still undergoes full BigDecimal/BigInteger conversion before the exactness check rejects it. If the stated intent is to bound conversion cost before `BigDecimal`, extend the check to non-string numeric primitives (e.g. reject oversized `Number` values before `getBigDecimal`), and confirm the Jackson number-length limit actually caps this path.</comment>

<file context>
@@ -433,12 +437,27 @@ public static String getHexString(final String string) {
+    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);
</file context>

&& ((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) {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down
18 changes: 11 additions & 7 deletions framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -422,12 +425,19 @@ private String mintParamsToHexString(GrpcAPI.ShieldedTRC20Parameters mintParams,
private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transferParams,
List<BytesMessage> spendAuthoritySignature,
boolean withAsk) {
List<ShieldContract.SpendDescription> spendDescs = transferParams.getSpendDescriptionList();
List<ShieldContract.ReceiveDescription> 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<ShieldContract.SpendDescription> spendDescs = transferParams.getSpendDescriptionList();
for (ShieldContract.SpendDescription spendDesc : spendDescs) {
input = ByteUtil.merge(input,
spendDesc.getNullifier().toByteArray(),
Expand All @@ -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();
Expand All @@ -458,7 +464,6 @@ private String transferParamsToHexString(GrpcAPI.ShieldedTRC20Parameters transfe
byte[] spendCountBytes = ByteUtil.longTo32Bytes(spendCount);
byte[] authOffsetBytes = ByteUtil.longTo32Bytes(192 + 32 + 320 * spendCount);

List<ShieldContract.ReceiveDescription> recvDescs = transferParams.getReceiveDescriptionList();
for (ShieldContract.ReceiveDescription recvDesc : recvDescs) {
output = ByteUtil.merge(output,
recvDesc.getNoteCommitment().toByteArray(),
Expand All @@ -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);
Expand Down
Loading
Loading