Skip to content

fix(evmrpc): bound and validate storageKeys in eth_getProof#3556

Merged
amir-deris merged 5 commits into
mainfrom
amir/con-338-eth-getproof-storageKeys
Jun 10, 2026
Merged

fix(evmrpc): bound and validate storageKeys in eth_getProof#3556
amir-deris merged 5 commits into
mainfrom
amir/con-338-eth-getproof-storageKeys

Conversation

@amir-deris

@amir-deris amir-deris commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

eth_getProof had two vulnerabilities in its storageKeys handling:

  • No length cap: There was no cap on the number of keys that could be sent, and it could cause issue with RPC server processing under concurrency.
  • No key validation: keys were accepted as raw byte strings via common.BytesToHash([]byte(key)) instead of being decoded as hex-encoded storage slots. This differs from GetStorageAt, which already uses decodeHash to reject malformed input.

Changes

  • MaxStorageKeysPerProof = 1000 — requests exceeding this limit are rejected before any store I/O (matches geth's EIP-1186 implementation)
  • Replace common.BytesToHash([]byte(key)) with decodeHash(key) — rejects non-hex strings and correctly interprets hex slot addresses, consistent with GetStorageAt
  • Update TestGetProof to use properly hex-encoded slot keys; add assertions that malformed keys and oversized key arrays are both rejected

@amir-deris amir-deris self-assigned this Jun 8, 2026
@amir-deris amir-deris changed the title Added validation for storage key, and the length of keys fix(evmrpc): bound and validate storageKeys in eth_getProof Jun 8, 2026
@cursor

cursor Bot commented Jun 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes RPC input handling for a proof endpoint; legitimate clients must send hex slots (not raw bytes), and very large key batches are now rejected.

Overview
eth_getProof now validates and bounds storageKeys before issuing proof queries.

Requests with more than MaxStorageKeysPerProof (1024) keys are rejected up front, avoiding unbounded store work under load. Each key is parsed with decodeHash (same as GetStorageAt) instead of treating the string as raw bytes, so malformed hex fails fast and slot encoding matches Ethereum expectations.

TestGetProof uses hex-encoded slots and asserts errors for invalid keys and oversized key lists.

Reviewed by Cursor Bugbot for commit 6f79f2a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 10, 2026, 9:25 AM

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.28%. Comparing base (ba465c7) to head (6f79f2a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3556      +/-   ##
==========================================
- Coverage   59.16%   58.28%   -0.89%     
==========================================
  Files        2226     2147      -79     
  Lines      183561   174604    -8957     
==========================================
- Hits       108613   101767    -6846     
+ Misses      65167    63773    -1394     
+ Partials     9781     9064     -717     
Flag Coverage Δ
sei-chain-pr 67.23% <100.00%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
evmrpc/state.go 64.17% <100.00%> (+5.12%) ⬆️

... and 144 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@amir-deris amir-deris requested review from bdchatham and masih June 8, 2026 18:13

@bdchatham bdchatham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a5e0b44. Configure here.

Comment thread evmrpc/state.go
Comment thread evmrpc/state.go Outdated
Comment thread evmrpc/state.go
@amir-deris amir-deris added this pull request to the merge queue Jun 10, 2026
Merged via the queue into main with commit b29c2ab Jun 10, 2026
55 checks passed
@amir-deris amir-deris deleted the amir/con-338-eth-getproof-storageKeys branch June 10, 2026 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants