Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sidebar/sidebar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const docs = [
{ text: "boolean-cst", link: "/forge/linting/boolean-cst" },
{ text: "dangerous-unary-operator", link: "/forge/linting/dangerous-unary-operator" },
{ text: "divide-before-multiply", link: "/forge/linting/divide-before-multiply" },
{ text: "ecrecover", link: "/forge/linting/ecrecover" },
{ text: "incorrect-erc20-interface", link: "/forge/linting/incorrect-erc20-interface" },
{ text: "incorrect-erc721-interface", link: "/forge/linting/incorrect-erc721-interface" },
{ text: "incorrect-strict-equality", link: "/forge/linting/incorrect-strict-equality" },
Expand Down
1 change: 1 addition & 0 deletions src/pages/forge/linting.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ navigation on the right to browse by severity.
- [`boolean-cst`](/forge/linting/boolean-cst) — Flags expressions where a boolean constant (`true`/`false`) is used as a control-flow condition or operand of a boolean operator, which usually indicates dead code or a leftover debug toggle.
- [`dangerous-unary-operator`](/forge/linting/dangerous-unary-operator) — Flags assignments whose `=` is fused to a unary operator (`=-`, `=~`), which parses as a plain assignment rather than the compound assignment it resembles.
- [`divide-before-multiply`](/forge/linting/divide-before-multiply) — Flags arithmetic expressions where division is performed before multiplication, which can cause unintended precision loss in integer arithmetic.
- [`ecrecover`](/forge/linting/ecrecover) — Flags direct calls to Solidity's `ecrecover` builtin when the signature's `s` value is not proven to be canonical.
- [`incorrect-erc20-interface`](/forge/linting/incorrect-erc20-interface) — Flags interfaces or contracts whose function signatures match an ERC20 method by name and parameters but use the wrong return type.
- [`incorrect-erc721-interface`](/forge/linting/incorrect-erc721-interface) — Flags interfaces or contracts whose function signatures match an ERC721 (or ERC165) method by name and parameters but use the wrong return type.
- [`locked-ether`](/forge/linting/locked-ether) — Flags contracts that can receive Ether (via `payable` functions, `receive()`, or a payable `fallback()`) but expose no code path that can send Ether out, permanently trapping user funds.
Expand Down
58 changes: 58 additions & 0 deletions src/pages/forge/linting/ecrecover.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
description: Unsafe use of Solidity's ecrecover builtin
---

## Unsafe ecrecover

**Severity**: `Med`
**ID**: `ecrecover`

Flags direct calls to Solidity's `ecrecover` builtin when the signature's `s` value is not proven
to be in the canonical lower half of the secp256k1 curve order.

### What it does

The lint follows simple local aliases and recognizes low-`s` bounds established by `require`,
`assert`, conditionals, and early-return or revert branches. It accepts the canonical maximum
`0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0`, stricter bounds, reversed
comparisons, and the equivalent strict comparison against that value plus one. Facts are
invalidated by reassignment, loop-carried writes, and calls that may change mutable state. They are
retained only when they hold on every path reaching the call.

Checks on `v`, the recovered address, nonces, domain separators, or the top bit of an EIP-2098
signature do not prove that `s` is canonical and do not suppress the warning.

The analysis is intentionally local and requires the low-`s` guard to be established before the
call. It does not summarize internal helpers or modifiers, prove post-call guards, recognize bound
signature schemes that fix one recovery ID, or inspect Yul and low-level calls to precompile
address `0x01`.

### Why is this bad?

For each valid high-`s` ECDSA signature, an attacker can derive a second signature for the same
message and signer by replacing `s` with its complement in the curve order and flipping `v`.
Contracts that use the signature bytes as a unique identifier can therefore have replay or
double-use protections bypassed. The `ecrecover` precompile does not enforce the low-`s` rule that
Ethereum transactions enforce.

### Example

#### Bad

```solidity
function recover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) pure returns (address) {
return ecrecover(hash, v, r, s);
}
```

#### Good

```solidity
uint256 constant HALF_ORDER =
0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0;

function recover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) pure returns (address) {
require(uint256(s) <= HALF_ORDER, "invalid signature s");
return ecrecover(hash, v, r, s);
}
```