From 18992c0ebc4bbb9218cafe712d6abade4303f9d1 Mon Sep 17 00:00:00 2001 From: vividctrlalt Date: Tue, 18 Aug 2026 18:00:34 +0000 Subject: [PATCH] fix(http): return clear errors for invalid API addresses --- .../services/http/EstimateEnergyServlet.java | 18 ++-- .../http/TriggerConstantContractServlet.java | 18 ++-- .../http/TriggerSmartContractServlet.java | 18 ++-- .../org/tron/core/services/http/Util.java | 79 ++++++++++++++++ .../http/EstimateEnergyServletTest.java | 82 +++++++++++++++++ .../TriggerConstantContractServletTest.java | 91 +++++++++++++++++++ .../tron/core/services/http/UtilMockTest.java | 34 +++++++ 7 files changed, 316 insertions(+), 24 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/services/http/EstimateEnergyServletTest.java diff --git a/framework/src/main/java/org/tron/core/services/http/EstimateEnergyServlet.java b/framework/src/main/java/org/tron/core/services/http/EstimateEnergyServlet.java index 91d673a2d08..b5164b9ba2c 100644 --- a/framework/src/main/java/org/tron/core/services/http/EstimateEnergyServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/EstimateEnergyServlet.java @@ -3,6 +3,7 @@ import com.google.protobuf.ByteString; import io.netty.util.internal.StringUtil; import java.io.IOException; +import java.security.InvalidParameterException; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -60,16 +61,17 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) wallet.estimateEnergy(build.build(), trxCap, trxExtBuilder, retBuilder, estimateEnergyBuilder); + } catch (InvalidParameterException e) { + Util.writeError(response, Return.response_code.OTHER_ERROR.name(), e.getMessage()); + return; } catch (ContractValidateException e) { - retBuilder.setResult(false).setCode(Return.response_code.CONTRACT_VALIDATE_ERROR) - .setMessage(ByteString.copyFromUtf8(e.getMessage())); + String message = e.getMessage() != null ? e.getMessage() : "contract validate error"; + Util.writeError(response, Return.response_code.CONTRACT_VALIDATE_ERROR.name(), message); + return; } catch (Exception e) { - String errString = null; - if (e.getMessage() != null) { - errString = e.getMessage().replaceAll("[\"]", "\'"); - } - retBuilder.setResult(false).setCode(Return.response_code.OTHER_ERROR) - .setMessage(ByteString.copyFromUtf8(e.getClass() + " : " + errString)); + logger.warn("internal error", e); + Util.writeError(response, Return.response_code.OTHER_ERROR.name(), Util.INTERNAL_ERROR_MSG); + return; } estimateEnergyBuilder.setResult(retBuilder); response.getWriter().println( diff --git a/framework/src/main/java/org/tron/core/services/http/TriggerConstantContractServlet.java b/framework/src/main/java/org/tron/core/services/http/TriggerConstantContractServlet.java index 634165911d1..2cf216aa63b 100644 --- a/framework/src/main/java/org/tron/core/services/http/TriggerConstantContractServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/TriggerConstantContractServlet.java @@ -3,6 +3,7 @@ import com.google.protobuf.ByteString; import io.netty.util.internal.StringUtil; import java.io.IOException; +import java.security.InvalidParameterException; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -67,16 +68,17 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) trx = Util.setTransactionExtraData(jsonObject, trx, visible); trxExtBuilder.setTransaction(trx); retBuilder.setResult(true).setCode(response_code.SUCCESS); + } catch (InvalidParameterException e) { + Util.writeError(response, response_code.OTHER_ERROR.name(), e.getMessage()); + return; } catch (ContractValidateException e) { - retBuilder.setResult(false).setCode(response_code.CONTRACT_VALIDATE_ERROR) - .setMessage(ByteString.copyFromUtf8(e.getMessage())); + String message = e.getMessage() != null ? e.getMessage() : "contract validate error"; + Util.writeError(response, response_code.CONTRACT_VALIDATE_ERROR.name(), message); + return; } catch (Exception e) { - String errString = null; - if (e.getMessage() != null) { - errString = e.getMessage().replaceAll("[\"]", "\'"); - } - retBuilder.setResult(false).setCode(response_code.OTHER_ERROR) - .setMessage(ByteString.copyFromUtf8(e.getClass() + " : " + errString)); + logger.warn("internal error", e); + Util.writeError(response, response_code.OTHER_ERROR.name(), Util.INTERNAL_ERROR_MSG); + return; } trxExtBuilder.setResult(retBuilder); response.getWriter().println(Util.printTransactionExtention(trxExtBuilder.build(), visible)); diff --git a/framework/src/main/java/org/tron/core/services/http/TriggerSmartContractServlet.java b/framework/src/main/java/org/tron/core/services/http/TriggerSmartContractServlet.java index bc4d9dc5f66..d2bb7a07e36 100644 --- a/framework/src/main/java/org/tron/core/services/http/TriggerSmartContractServlet.java +++ b/framework/src/main/java/org/tron/core/services/http/TriggerSmartContractServlet.java @@ -41,6 +41,7 @@ private void validateParameter(String contract) { if (StringUtil.isNullOrEmpty(jsonObject.getString(Util.CONTRACT_ADDRESS))) { throw new InvalidParameterException(Util.CONTRACT_ADDRESS + " isn't set."); } + Util.validateAddressesAndHex(jsonObject, Util.getVisiblePost(contract)); } protected void doPost(HttpServletRequest request, HttpServletResponse response) @@ -85,16 +86,17 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) trx = Util.setTransactionPermissionId(jsonObject, trx); trxExtBuilder.setTransaction(trx); retBuilder.setResult(true).setCode(response_code.SUCCESS); + } catch (InvalidParameterException e) { + Util.writeError(response, response_code.OTHER_ERROR.name(), e.getMessage()); + return; } catch (ContractValidateException e) { - retBuilder.setResult(false).setCode(response_code.CONTRACT_VALIDATE_ERROR) - .setMessage(ByteString.copyFromUtf8(e.getMessage())); + String message = e.getMessage() != null ? e.getMessage() : "contract validate error"; + Util.writeError(response, response_code.CONTRACT_VALIDATE_ERROR.name(), message); + return; } catch (Exception e) { - String errString = null; - if (e.getMessage() != null) { - errString = e.getMessage().replaceAll("[\"]", "\'"); - } - retBuilder.setResult(false).setCode(response_code.OTHER_ERROR) - .setMessage(ByteString.copyFromUtf8(e.getClass() + " : " + errString)); + logger.warn("internal error", e); + Util.writeError(response, response_code.OTHER_ERROR.name(), Util.INTERNAL_ERROR_MSG); + return; } trxExtBuilder.setResult(retBuilder); response.getWriter().println(Util.printTransactionExtention(trxExtBuilder.build(), visible)); 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..ad98f187be7 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 @@ -85,6 +85,11 @@ public class Util { public static final String FUNCTION_PARAMETER = "parameter"; public static final String CALL_DATA = "data"; + public static final String INVALID_ADDRESS_BASE58CHECK = + "invalid address: base58check failed"; + public static final String INVALID_HEX_LENGTH = "invalid hex: length must be even"; + public static final String INTERNAL_ERROR_MSG = "internal error"; + public static boolean hasMeaningfulEvents(ProtocolStringList events) { return events.stream().anyMatch(s -> !s.isEmpty()); } @@ -661,6 +666,80 @@ public static void validateParameter(String contract) throws InvalidParameterExc throw new InvalidParameterException("While trying to deploy, " + FUNCTION_SELECTOR + " and " + CALL_DATA + " can not be both set."); } + boolean visible = false; + if (jsonObject.containsKey(VISIBLE)) { + visible = Boolean.parseBoolean(jsonObject.getString(VISIBLE)); + } + validateAddressesAndHex(jsonObject, visible); + } + + /** + * Validate address encoding and hex payload length for TVM HTTP APIs. + * Base58Check addresses are checked when {@code visible} is true; hex + * fields must have even length. + */ + public static void validateAddressesAndHex(JSONObject jsonObject, boolean visible) { + validateAddressValue(jsonObject.getString(OWNER_ADDRESS), visible); + validateAddressValue(jsonObject.getString(CONTRACT_ADDRESS), visible); + validateHexString(jsonObject.getString(FUNCTION_PARAMETER)); + validateHexString(jsonObject.getString(CALL_DATA)); + } + + /** + * Validate a single address. Empty values are ignored so required-field + * checks can remain the caller's responsibility. + */ + public static void validateAddressValue(String address, boolean visible) { + if (StringUtils.isEmpty(address)) { + return; + } + if (visible) { + byte[] decoded; + try { + decoded = decodeFromBase58Check(address); + } catch (IllegalArgumentException e) { + throw new InvalidParameterException(INVALID_ADDRESS_BASE58CHECK); + } + if (decoded == null) { + throw new InvalidParameterException(INVALID_ADDRESS_BASE58CHECK); + } + } else if ((address.length() & 1) != 0) { + throw new InvalidParameterException(INVALID_HEX_LENGTH); + } + } + + /** + * Reject odd-length hex so {@code ByteArray.fromHexString} cannot NPE. + */ + public static void validateHexString(String hex) { + if (StringUtils.isEmpty(hex)) { + return; + } + String value = hex; + if (value.length() >= 2 + && (value.startsWith("0x") || value.startsWith("0X"))) { + value = value.substring(2); + } + if ((value.length() & 1) != 0) { + throw new InvalidParameterException(INVALID_HEX_LENGTH); + } + } + + /** + * Write a sanitized HTTP API error. Parameter errors pass a stable + * client-facing message; internal errors must use {@link #INTERNAL_ERROR_MSG} + * and never include {@code e.getClass()} or {@code e.getMessage()}. + */ + public static void writeError(HttpServletResponse response, String code, String message) + throws IOException { + String safeMessage = message == null ? INTERNAL_ERROR_MSG : message; + JSONObject result = new JSONObject(); + result.put("result", false); + result.put("code", code); + result.put("message", safeMessage); + JSONObject json = new JSONObject(); + json.put("result", result); + response.getWriter().println(json.toJSONString()); } public static String getJsonString(String str) { diff --git a/framework/src/test/java/org/tron/core/services/http/EstimateEnergyServletTest.java b/framework/src/test/java/org/tron/core/services/http/EstimateEnergyServletTest.java new file mode 100644 index 00000000000..35434446f1a --- /dev/null +++ b/framework/src/test/java/org/tron/core/services/http/EstimateEnergyServletTest.java @@ -0,0 +1,82 @@ +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.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletResponse; +import org.tron.common.crypto.ECKey; +import org.tron.common.utils.ByteArray; + +public class EstimateEnergyServletTest extends BaseHttpTest { + + private static final String ISSUE_OWNER = "TKgD8Qnx9Zw3RNjdiU2i5y2Swa2y4QvG6v"; + private static final String USDT = "TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t"; + private static final String ODD_HEX = + "0000000000000000000000418a8e8b8c8d8e8f9a9b9c9d9e9f0a1b2c3d4e5f6"; + + private EstimateEnergyServlet servlet; + + @Override + protected void setUpMocks() throws Exception { + servlet = new EstimateEnergyServlet(); + injectWallet(servlet); + } + + @Test + public void testInvalidBase58CheckOwnerReturnsStableError() throws Exception { + String body = "{\"owner_address\":\"" + ISSUE_OWNER + "\"," + + "\"contract_address\":\"" + USDT + "\"," + + "\"function_selector\":\"isBlackListed(address)\"," + + "\"parameter\":\"00\"," + + "\"visible\":true}"; + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INVALID_ADDRESS_BASE58CHECK)); + assertFalse(content.contains("NullPointerException")); + assertFalse(content.contains("java.lang")); + verify(wallet, never()).estimateEnergy(any(), any(), any(), any(), any()); + } + + @Test + public void testOddLengthHexParameterReturnsStableError() throws Exception { + String owner = ByteArray.toHexString(new ECKey().getAddress()); + String contract = ByteArray.toHexString(new ECKey().getAddress()); + String body = "{\"owner_address\":\"" + owner + "\"," + + "\"contract_address\":\"" + contract + "\"," + + "\"function_selector\":\"isBlackListed(address)\"," + + "\"parameter\":\"" + ODD_HEX + "\"}"; + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INVALID_HEX_LENGTH)); + assertFalse(content.contains("NullPointerException")); + verify(wallet, never()).estimateEnergy(any(), any(), any(), any(), any()); + } + + @Test + public void testInternalErrorDoesNotLeakExceptionDetails() throws Exception { + String owner = ByteArray.toHexString(new ECKey().getAddress()); + String contract = ByteArray.toHexString(new ECKey().getAddress()); + String body = "{\"owner_address\":\"" + owner + "\"," + + "\"contract_address\":\"" + contract + "\"," + + "\"data\":\"00\"}"; + when(wallet.createTransactionCapsule(any(), any())) + .thenThrow(new NullPointerException("secret internals")); + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INTERNAL_ERROR_MSG)); + assertFalse(content.contains("NullPointerException")); + assertFalse(content.contains("secret internals")); + } +} diff --git a/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java b/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java index 2a139f8a158..826fb066123 100644 --- a/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java +++ b/framework/src/test/java/org/tron/core/services/http/TriggerConstantContractServletTest.java @@ -1,7 +1,10 @@ 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.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -13,6 +16,13 @@ public class TriggerConstantContractServletTest extends BaseHttpTest { + private static final String ISSUE_OWNER = "TKgD8Qnx9Zw3RNjdiU2i5y2Swa2y4QvG6v"; + private static final String USDT = "TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t"; + private static final String ODD_HEX = + "0000000000000000000000418a8e8b8c8d8e8f9a9b9c9d9e9f0a1b2c3d4e5f6"; + private static final String EVEN_HEX = + "0000000000000000000000418a8e8b8c8d8e8f9a9b9c9d9e9f0a1b2c3d4e5f60"; + private TriggerConstantContractServlet servlet; @Override @@ -46,4 +56,85 @@ public void testManyFlatFieldsDoesNotOverflowStack() throws Exception { assertEquals(200, response.getStatus()); verify(wallet).triggerConstantContract(any(), any(), any(), any()); } + + @Test + public void testInvalidBase58CheckOwnerReturnsStableError() throws Exception { + String body = "{\"owner_address\":\"" + ISSUE_OWNER + "\"," + + "\"contract_address\":\"" + USDT + "\"," + + "\"function_selector\":\"isBlackListed(address)\"," + + "\"parameter\":\"" + EVEN_HEX + "\"," + + "\"visible\":true}"; + + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INVALID_ADDRESS_BASE58CHECK)); + assertFalse(content.contains("NullPointerException")); + assertFalse(content.contains("java.lang")); + verify(wallet, never()).triggerConstantContract(any(), any(), any(), any()); + } + + @Test + public void testOddLengthHexParameterReturnsStableError() throws Exception { + String owner = ByteArray.toHexString(new ECKey().getAddress()); + String contract = ByteArray.toHexString(new ECKey().getAddress()); + String body = "{\"owner_address\":\"" + owner + "\"," + + "\"contract_address\":\"" + contract + "\"," + + "\"function_selector\":\"isBlackListed(address)\"," + + "\"parameter\":\"" + ODD_HEX + "\"}"; + + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INVALID_HEX_LENGTH)); + assertFalse(content.contains("NullPointerException")); + assertFalse(content.contains("java.lang")); + verify(wallet, never()).triggerConstantContract(any(), any(), any(), any()); + } + + @Test + public void testIssuePayloadReturnsParameterErrorNotNpe() throws Exception { + String body = "{\"owner_address\":\"" + ISSUE_OWNER + "\"," + + "\"contract_address\":\"" + USDT + "\"," + + "\"function_selector\":\"isBlackListed(address)\"," + + "\"parameter\":\"" + ODD_HEX + "\"," + + "\"visible\":true}"; + + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INVALID_ADDRESS_BASE58CHECK) + || content.contains(Util.INVALID_HEX_LENGTH)); + assertFalse(content.contains("NullPointerException")); + assertFalse(content.contains("java.lang")); + verify(wallet, never()).createTransactionCapsule(any(), any()); + } + + @Test + public void testInternalErrorDoesNotLeakExceptionDetails() throws Exception { + String owner = ByteArray.toHexString(new ECKey().getAddress()); + String contract = ByteArray.toHexString(new ECKey().getAddress()); + String body = "{\"owner_address\":\"" + owner + "\"," + + "\"contract_address\":\"" + contract + "\"," + + "\"data\":\"00\"}"; + + when(wallet.createTransactionCapsule(any(), any())) + .thenThrow(new NullPointerException("secret internals")); + + MockHttpServletResponse response = newResponse(); + servlet.doPost(postRequest(body), response); + + assertEquals(200, response.getStatus()); + String content = response.getContentAsString(); + assertTrue(content.contains(Util.INTERNAL_ERROR_MSG)); + assertFalse(content.contains("NullPointerException")); + assertFalse(content.contains("secret internals")); + assertFalse(content.contains("java.lang")); + } } diff --git a/framework/src/test/java/org/tron/core/services/http/UtilMockTest.java b/framework/src/test/java/org/tron/core/services/http/UtilMockTest.java index d4124c90adf..6fbc0962cfd 100644 --- a/framework/src/test/java/org/tron/core/services/http/UtilMockTest.java +++ b/framework/src/test/java/org/tron/core/services/http/UtilMockTest.java @@ -12,6 +12,7 @@ import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; +import org.springframework.mock.web.MockHttpServletResponse; import org.tron.api.GrpcAPI; import org.tron.common.utils.Sha256Hash; import org.tron.core.capsule.BlockCapsule; @@ -384,4 +385,37 @@ public void testGetJsonString() { Assert.assertEquals(expect, ret2); } + @Test + public void testValidateParameterInvalidBase58Check() { + String contract = "{\"owner_address\":\"TKgD8Qnx9Zw3RNjdiU2i5y2Swa2y4QvG6v\"," + + " \"contract_address\":\"TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t\"," + + " \"visible\":true}"; + InvalidParameterException ex = Assert.assertThrows( + InvalidParameterException.class, + () -> Util.validateParameter(contract)); + Assert.assertEquals(Util.INVALID_ADDRESS_BASE58CHECK, ex.getMessage()); + } + + @Test + public void testValidateParameterOddLengthHex() { + String contract = "{\"owner_address\":\"41548794500882809695a8a687866e76d4271a1abc\"," + + " \"contract_address\":\"41abd4b9367799eaa3197fecb144eb71de1e049150\"," + + " \"parameter\":\"abc\"}"; + InvalidParameterException ex = Assert.assertThrows( + InvalidParameterException.class, + () -> Util.validateParameter(contract)); + Assert.assertEquals(Util.INVALID_HEX_LENGTH, ex.getMessage()); + } + + @Test + public void testWriteErrorSanitizesPayload() throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + Util.writeError(response, "OTHER_ERROR", Util.INVALID_ADDRESS_BASE58CHECK); + String content = response.getContentAsString(); + Assert.assertTrue(content.contains(Util.INVALID_ADDRESS_BASE58CHECK)); + Assert.assertTrue(content.contains("OTHER_ERROR")); + Assert.assertFalse(content.contains("java.lang")); + Assert.assertFalse(content.contains("NullPointerException")); + } + }