From f09428f0486807353378f6fbf77e2a80cd48189e Mon Sep 17 00:00:00 2001 From: jsvisa Date: Tue, 16 Sep 2025 16:22:37 +0800 Subject: [PATCH 1/3] fix(ethapi): check storage key in the rpc framework --- internal/ethapi/api.go | 67 +++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index d7cf47468c4..0af6656a03c 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -19,10 +19,12 @@ package ethapi import ( "context" "encoding/hex" + "encoding/json" "errors" "fmt" gomath "math" "math/big" + "reflect" "strings" "time" @@ -365,20 +367,8 @@ func (n *proofList) Delete(key []byte) error { } // GetProof returns the Merkle-proof for a given account and optionally some storage keys. -func (api *BlockChainAPI) GetProof(ctx context.Context, address common.Address, storageKeys []string, blockNrOrHash rpc.BlockNumberOrHash) (*AccountResult, error) { - var ( - keys = make([]common.Hash, len(storageKeys)) - keyLengths = make([]int, len(storageKeys)) - storageProof = make([]StorageResult, len(storageKeys)) - ) - // Deserialize all keys. This prevents state access on invalid input. - for i, hexKey := range storageKeys { - var err error - keys[i], keyLengths[i], err = decodeHash(hexKey) - if err != nil { - return nil, err - } - } +func (api *BlockChainAPI) GetProof(ctx context.Context, address common.Address, storageKeys []StorageKey, blockNrOrHash rpc.BlockNumberOrHash) (*AccountResult, error) { + storageProof := make([]StorageResult, len(storageKeys)) statedb, header, err := api.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if statedb == nil || err != nil { return nil, err @@ -386,7 +376,7 @@ func (api *BlockChainAPI) GetProof(ctx context.Context, address common.Address, codeHash := statedb.GetCodeHash(address) storageRoot := statedb.GetStorageRoot(address) - if len(keys) > 0 { + if len(storageKeys) > 0 { var storageTrie state.Trie if storageRoot != types.EmptyRootHash && storageRoot != (common.Hash{}) { id := trie.StorageTrieID(header.Root, crypto.Keccak256Hash(address.Bytes()), storageRoot) @@ -397,13 +387,14 @@ func (api *BlockChainAPI) GetProof(ctx context.Context, address common.Address, storageTrie = st } // Create the proofs for the storageKeys. - for i, key := range keys { + for i, storageKey := range storageKeys { + key := storageKey.Hash() // Output key encoding is a bit special: if the input was a 32-byte hash, it is // returned as such. Otherwise, we apply the QUANTITY encoding mandated by the // JSON-RPC spec for getProof. This behavior exists to preserve backwards // compatibility with older client versions. var outputKey string - if keyLengths[i] != 32 { + if storageKey.InputLength() != 32 { outputKey = hexutil.EncodeBig(key.Big()) } else { outputKey = hexutil.Encode(key[:]) @@ -581,19 +572,49 @@ func (api *BlockChainAPI) GetCode(ctx context.Context, address common.Address, b return code, state.Error() } +// StorageKey represents a storage key that can be unmarshalled from hex strings +// of varying lengths (up to 32 bytes / 64 hex characters). +type StorageKey struct { + hash common.Hash + length int +} + +// UnmarshalJSON implements json.Unmarshaler for StorageKey. +func (s *StorageKey) UnmarshalJSON(input []byte) error { + // Check if input is a JSON string + if len(input) < 2 || input[0] != '"' || input[len(input)-1] != '"' { + return &json.UnmarshalTypeError{Value: "non-string", Type: reflect.TypeFor[StorageKey]()} + } + // Remove quotes from JSON string + hexStr := string(input[1 : len(input)-1]) + hash, length, err := decodeHash(hexStr) + if err != nil { + return fmt.Errorf("unable to decode storage key: %s", err) + } + s.hash = hash + s.length = length + return nil +} + +// Hash returns the underlying common.Hash. +func (s StorageKey) Hash() common.Hash { + return s.hash +} + +// InputLength returns the length in bytes of the original hex input. +func (s StorageKey) InputLength() int { + return s.length +} + // GetStorageAt returns the storage from the state at the given address, key and // block number. The rpc.LatestBlockNumber and rpc.PendingBlockNumber meta block // numbers are also allowed. -func (api *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address, hexKey string, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) { +func (api *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address, key StorageKey, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) { state, _, err := api.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return nil, err } - key, _, err := decodeHash(hexKey) - if err != nil { - return nil, fmt.Errorf("unable to decode storage key: %s", err) - } - res := state.GetState(address, key) + res := state.GetState(address, key.Hash()) return res[:], state.Error() } From 65a5fe54ef89e325231d1983d6d0d8edbbb2b002 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Tue, 16 Sep 2025 16:23:20 +0800 Subject: [PATCH 2/3] add test cases --- internal/ethapi/api_test.go | 164 ++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index aaa002b5ec0..bff5f002843 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -4052,5 +4052,169 @@ func TestSendRawTransactionSync_Timeout(t *testing.T) { } if got, want := de.ErrorData(), tx.Hash().Hex(); got != want { t.Fatalf("expected ErrorData=%s, got %v", want, got) + + } +} + +func TestStorageKeyUnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + expected common.Hash + expectedLength int + expectedError bool + }{ + { + name: "short hex with 0x prefix", + input: `"0x1"`, + expected: common.HexToHash("0x1"), + expectedLength: 1, + }, + { + name: "short hex without 0x prefix", + input: `"1"`, + expected: common.HexToHash("0x1"), + expectedLength: 1, + }, + { + name: "two byte hex", + input: `"0x1234"`, + expected: common.HexToHash("0x1234"), + expectedLength: 2, + }, + { + name: "four byte hex", + input: `"0x12345678"`, + expected: common.HexToHash("0x12345678"), + expectedLength: 4, + }, + { + name: "full 32-byte hash with 0x prefix", + input: `"0x0000000000000000000000000000000000000000000000000000000000000001"`, + expected: common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000001"), + expectedLength: 32, + }, + { + name: "full 32-byte hash without 0x prefix", + input: `"0000000000000000000000000000000000000000000000000000000000000001"`, + expected: common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000001"), + expectedLength: 32, + }, + { + name: "odd length hex (gets padded)", + input: `"0x123"`, + expected: common.HexToHash("0x0123"), + expectedLength: 2, // After padding "123" becomes "0123" which is 2 bytes + }, + { + name: "odd length without prefix (gets padded)", + input: `"123"`, + expected: common.HexToHash("0x0123"), + expectedLength: 2, // After padding "123" becomes "0123" which is 2 bytes + }, + { + name: "zero value", + input: `"0x0"`, + expected: common.HexToHash("0x0"), + expectedLength: 1, + }, + { + name: "zero value without prefix", + input: `"0"`, + expected: common.HexToHash("0x0"), + expectedLength: 1, + }, + { + name: "empty hex string", + input: `"0x"`, + expected: common.Hash{}, + expectedLength: 0, + }, + { + name: "uppercase hex", + input: `"0xDEADBEEF"`, + expected: common.HexToHash("0xDEADBEEF"), + expectedLength: 4, + }, + { + name: "mixed case hex", + input: `"0xDeAdBeEf"`, + expected: common.HexToHash("0xDeAdBeEf"), + expectedLength: 4, + }, + // Error cases + { + name: "invalid hex characters", + input: `"0xGG"`, + expectedError: true, + }, + { + name: "non-string input", + input: `123`, + expectedError: true, + }, + { + name: "null input", + input: `null`, + expectedError: true, + }, + { + name: "boolean input", + input: `true`, + expectedError: true, + }, + { + name: "array input", + input: `[]`, + expectedError: true, + }, + { + name: "object input", + input: `{}`, + expectedError: true, + }, + { + name: "unterminated string", + input: `"0x123`, + expectedError: true, + }, + { + name: "empty string", + input: `""`, + expected: common.Hash{}, + expectedLength: 0, + }, + { + name: "hex string too long (more than 32 bytes)", + input: `"0x00000000000000000000000000000000000000000000000000000000000000001"`, // 33 bytes + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var key StorageKey + err := json.Unmarshal([]byte(tt.input), &key) + + if tt.expectedError { + if err == nil { + t.Errorf("expected error but got none") + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + if key.Hash() != tt.expected { + t.Errorf("hash mismatch: got %x, want %x", key.Hash(), tt.expected) + } + + if key.InputLength() != tt.expectedLength { + t.Errorf("length mismatch: got %d, want %d", key.InputLength(), tt.expectedLength) + } + }) } } From 5bce00e9d91bdcaffc59382be5981a3ec014f6fd Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 23 Oct 2025 11:11:13 +0800 Subject: [PATCH 3/3] gofmt --- internal/ethapi/api_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index bff5f002843..38edae2cb5d 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -4052,7 +4052,6 @@ func TestSendRawTransactionSync_Timeout(t *testing.T) { } if got, want := de.ErrorData(), tx.Hash().Hex(); got != want { t.Fatalf("expected ErrorData=%s, got %v", want, got) - } }