feat(lint): detect unsafe ecrecover#15765
Open
mattsse wants to merge 5 commits into
Open
Conversation
Track canonical low-s checks through local control flow and aliases so direct ecrecover calls warn only when signature malleability is not ruled out.
Invalidate low-s facts across loop backedges and calls that may mutate state. Preserve simultaneous assignment semantics, wrapped constants, and control-flow joins so safe calls stay quiet without hiding reachable high-s calls.
mattsse
requested review from
0xrusowsky,
DaniPopes,
figtracer,
grandizzy,
mablr and
stevencartavia
as code owners
July 15, 2026 14:35
mattsse
marked this pull request as draft
July 15, 2026 16:10
mattsse
marked this pull request as ready for review
July 15, 2026 16:52
94 tasks
mablr
previously approved these changes
Jul 15, 2026
figtracer
reviewed
Jul 15, 2026
Defer diagnostics for ecrecover results stored locally until the result is read or a matching low-s guard validates the signature value. Preserve pending results across control-flow joins so partial guards remain unsafe.
mablr
reviewed
Jul 16, 2026
Follow only feasible constant branches and propagate selected exits so unreachable ecrecover calls are ignored. Document supported post-call low-s guards.
mablr
reviewed
Jul 16, 2026
Prune infeasible expression arms when tracking calls and low-s facts. Carry deferred recoveries through tuple and nested local assignments while treating observable state writes as immediate uses.
| return ControlFlow::Continue(()); | ||
| } | ||
| StmtKind::Err(_) | StmtKind::AssemblyBlock(_) => { | ||
| self.state = FlowState::default(); |
Member
There was a problem hiding this comment.
Resetting FlowState here also drops deferred pending recoveries, allowing an assembly block to suppress an expected ecrecover warning. For example, address signer = ecrecover(...); assembly { pop(0) } return signer; currently produces no warning. I reproduced this with unrelated, empty, and memory-safe assembly blocks; moving the same assembly before ecrecover warns correctly. One option would be to treat AssemblyBlock as an opaque boundary and preserve or conservatively report unresolved pending recoveries instead of clearing them, while keeping overwritten recoveries clean.
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
4 participants
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.
This adds the medium-severity
ecrecoverlint from #14381. It reports direct uses of Solidity's builtin unless the signature'ssvalue is proven to be in the canonical lower half of the secp256k1 order, with path-sensitive handling for guards, aliases, branches, loops, assignments, mutable calls, and Solidity constant expressions.A locally stored recovery is deferred until its first observable use, so a dominating post-call low-s guard can validate the exact signature value before the signer affects behavior. Pending recoveries survive joins when any path remains unguarded, while pre-guard reads, state writes, reassigned signature values, and implicit named-return uses still report. The UI fixture also covers comparison polarity, control-flow exits, tuple and ternary assignments, loop-carried writes, mutable-state invalidation, builtin resolution, and safe counterexamples.
The companion Book documentation is in foundry-rs/book#1981, with the post-call guard follow-up in foundry-rs/book#1994. This PR was implemented with Codex assistance across code, tests, and lint documentation, then independently reviewed and corrected by a fresh Codex context.