fix(jsonrpc): harden RPC/HTTP parameter validation#6828
Open
0xbigapple wants to merge 2 commits into
Open
Conversation
Add length/format guards across the JSON-RPC and HTTP address-handling paths so
malformed or oversized parameters are rejected early with a clear -32602 / error
response, instead of triggering O(n^2) work or leaking NPE / Internal errors.
- Commons.decodeFromBase58Check: reject non-34-char input before the O(n^2)
Base58 decode; single funnel covering every HTTP address input.
- JsonFormat.unescapeBytesSelfType: turn an invalid Base58 address (null or
illegal-char) into a clear "invalid address" error instead of a bare
ByteString.copyFrom(null) NPE; covers all merge-path address fields.
- JsonRpcApiUtil.addressCompatibleToByteArray: cap length at ADDRESS_SIZE + 2
(the +2 keeps 0x-prefixed 21-byte addresses valid) before fromHexString.
- JsonRpcApiUtil.parseBlockNumber: tighten MAX_BLOCK_NUM_HEX_LEN 100 -> 20 and
route the (String, Wallet) overload through the single-arg parser so it also
gets the length cap, overflow (longValueExact) and negative checks.
- TronJsonRpcImpl.getStorageAt: bound the storage key and parse it before the
contract lookup; wrap DataWord parsing so a >32-byte key
returns invalid-params instead of an Internal error.
- Hoist hashToByteArray/HASH_REGEX into JsonRpcApiUtil so LogFilterWrapper's
blockHash gets the same 32-byte-hash validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds length/format validation to several JSON-RPC and HTTP address-handling parameters, so
malformed or oversized input is rejected early instead of triggering
O(n²)work or leakingNullPointerException/-32001(json4j default error). Valid input is unaffected.Commons.decodeFromBase58Check— reject input whose length!= 34before theO(n²)Base58.decode; single funnel for every HTTP address input.JsonFormat.unescapeBytesSelfType— an invalid Base58 address (null / illegal char) now returns a clearinvalid addresserror instead of a bareByteString.copyFrom(null)NPE; covers all merge-path address fields.JsonRpcApiUtil.addressCompatibleToByteArray— cap length atADDRESS_SIZE + 2(the+2keeps0x-prefixed 21-byte addresses valid) beforefromHexString.JsonRpcApiUtil.parseBlockNumber— tightenMAX_BLOCK_NUM_HEX_LEN100 → 20, and route the(String, Wallet)overload through the single-arg parser so it also gets the length cap checks.TronJsonRpcImpl.getStorageAt— bound the storage key and parse it before the contract lookup; a>32-byte key now returns-32602 invalid storage key valueinstead of-32001 (json4j default error).LogFilterWrapper— validateblockHashvia the sharedJsonRpcApiUtil.hashToByteArray(moved here fromTronJsonRpcImpl).HASH_REGEXaccepts only hex digits[0-9a-fA-F], so a non-hex hash is rejected asinvalid hash valuerather than slipping through tofromHexString.Why are these changes required?
Several unauthenticated API entry points accepted unbounded / malformed input, causing two classes of problem:
decodeFromBase58Checkfed the whole string into theO(n²)Base58.decode; since the cost grows quadratically with length, a single oversized addresspayload (well within the 4 MB body cap) can tie up a request thread far out of proportion to
its size — a cheap DoS the body cap and rate limiter don't stop. Block-number parsing likewise
had no effective length bound before the
O(n²)BigIntegerconstructor.bare
NullPointerException;eth_getStorageAtsurfaced aRuntimeExceptionas-32001 Data word can't exceed 32 bytes: xxxinstead of-32602 invalid params;This PR has been tested by:
Follow up
Extra details
Representative affected endpoints:
eth_getStorageAthashToByteArray)ADDRESS_SIZE + 2)/wallet/*— all covered by the Base58 length/DoS guard, but they differ on an invalid address:Util.getHexAddressand just return{}(e.g.getcontract,`visible=true) — unchanged;getaccount,visible=true) get the NPE →invalid addressfix;Util.getAddress(request)(e.g.getReward) get null for an invalid address and degrade toreward 0.