Add erc7562-banned-opcodes detector for ERC-4338 validation functions - #3096
Open
saintparish4 wants to merge 2 commits into
Open
saintparish4 wants to merge 2 commits into
saintparish4 wants to merge 2 commits into
Conversation
ERC-7562 bans opcodes whose value can differ between a bundler'simulation and inclusion. Bundlers enforce the rules off-chain, in a tracer, so a contract that breaks them compiles, passes its own tests, passes a direct `EntryPoint.handleOps call` and is rejected only once it is deployed. The detector walks internal calls, library calls and modifiers out from validateUserOp and validatePaymasterUserOp, and reports every OP-011 opcode it reaches, plus BALANCE carrying its OP-080 staked-entity condition. Entry points are matched by name, so a contract that declares its own interface with an argument list no ERC-4337 version uses is still checked. Findings are merged across the FunctionContract copies Slither builds for each inheriting contract, so an inherited validation function is reported once, naming every deployable contract that has it. GAS, CREATE and CREATE2 are left out: each is permitted in contexts that need data flow to tell apart, and flagging every gasleft() would make the detector unusable. Fixtures and snapshot tests cover solc 0.5.16 and 0.8.26.
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.
Adds
erc7562-banned-opcodes, which reports ERC-7562banned opcodes reachable from an ERC-4337 validation function. Medium impact, High confidence.
Why
ERC-7562 is enforced by bundlers, off-chain, in a tracer — nothing on-chain enforces it. A
paymaster that reads
block.timestampduring validation compiles, passes its tests, passes areal
EntryPoint.handleOpscall, and is then dropped by every bundler in production, after ithas been deployed and staked. The reference implementation says as much in its own NatSpec, in
three separate interfaces: "Note that the validation code cannot use block.timestamp (or
block.number) directly." Nothing a developer runs before deploying checks it.
tx-originalready flags the ORIGIN case when the read sits in a conditional, for its ownphishing reason. The other eleven opcodes have no coverage in this context, and no detector
models "reachable from a validation entry point", which is what decides whether the contract
works at all.
What it does
Starting from
validateUserOpandvalidatePaymasterUserOp, follow internal calls, librarycalls and modifiers, and report each banned opcode reached:
GASPRICE, BLOCKHASH, BLOBHASH and SELFDESTRUCT, in both their Solidity and inline-assembly
spellings, and including the pre-0.7
nowandblock.difficulty.entities.
Entry points are matched by name rather than by
IAccount/IPaymasterinheritance, so acontract that declares its own interface is still checked. Slither builds a separate
FunctionContractfor every contract that inherits a function, so findings are merged: aninherited validation function is reported once, listing the deployable contracts it belongs to.
GAS (OP-012), CREATE (OP-032) and CREATE2 (OP-031) are deliberately out of scope. Each is
permitted in contexts that need data flow to distinguish, and flagging every
gasleft()is howa detector gets switched off. The EXTCODE* and *CALL rules (OP-041 to OP-062) depend on the
callee's deployment state, which is not visible statically.
Validation
Two corpora at pinned commits, compiled with plain solc.
No false positives on the audited reference implementation.
eth-infinitism/account-abstractionv0.8.0 (4cbc060), all 53 contract files: 52 compile, andevery one of them reports 0 findings. The remaining file needs
@uniswap/v3-peripheryand hasno validation function in it. The clean results include every file that defines or inherits an
entry point —
core/BaseAccount.sol,core/BasePaymaster.sol,accounts/SimpleAccount.sol,test/TestExpiryAccount.sol,test/TestPaymasterAcceptAll.soland nine others.TestPaymasterAcceptAllis the informative one: it readstx.originin its constructor, whicha contract-level check would report and this does not.
A real deviation in production code.
pimlicolabs/singleton-paymaster(1e2305d), threefiles, one finding each: an ORIGIN read enforcing a bundler allowlist, reached through one
internal call from the entry point. The V8 compilation also exercises the merge — V8 inherits
V7's validation function and both are deployable, so a single finding names both contracts.
That is also a fair statement of the detector's limitation: it reports reachability and cannot
see that the read is optional. Path sensitivity would turn this into a conditional finding, and
is the obvious next step rather than something for a first version.
Tests
Fixtures and snapshots on two compilers. 0.8.26 covers the modern spellings, including
blobhashandblock.blobbasefee, which need Cancun and so will not build under therepository's usual 0.8.20. 0.5.16 covers
now,block.difficulty, and a pre-0.6 contract thatis abstract without the keyword, with the finding landing on its concrete child.
pytest tests/e2e/detectors→ 386 passed.