feat(rules): [RULE] Rule B004: Detect Zero-Address Comparisons in Recovered Signatures (#826)

[jotel-dev]

[RULE] Rule B004: Detect Zero-Address Comparisons in Recovered Signatures (#826)

📌 Overview & Motivation

Closes #826

When Ethereum's native ecrecover precompile/built-in function processes malformed or invalid signature parameters (such as invalid v, r, or s values outside the curve order), it fails silently and returns address(0) instead of reverting.

If a smart contract compares this recovered address directly against:

1. An uninitialized state variable (which defaults to address(0)),
2. An uninitialized local memory/storage pointer, or
3. An empty/zero-address parameter,

the comparison evaluates to true, resulting in a critical signature authorization bypass.

This PR introduces static analysis rule B004 (ecrecover-zero-address) to inspect Solidity ASTs, detect unguarded ecrecover invocations, and enforce explicit address(0) validations or recommend OpenZeppelin's ECDSA.recover.

---

🔍 Vulnerability Mechanism & Real-World Impact

// ❌ VULNERABLE: If `expectedSigner` is uninitialized or address(0), any malformed signature bypasses authorization
function execute(bytes32 hash, uint8 v, bytes32 r, bytes32 s, address expectedSigner) external {
    address recovered = ecrecover(hash, v, r, s);
    require(recovered == expectedSigner, "Unauthorized"); // ecrecover failure returns address(0) == address(0) => PASS!
}

// ✅ SECURE: Explicit non-zero validation blocks uninitialized bypasses
function executeGuarded(bytes32 hash, uint8 v, bytes32 r, bytes32 s, address expectedSigner) external {
    address recovered = ecrecover(hash, v, r, s);
    require(recovered != address(0), "Invalid signature");
    require(recovered == expectedSigner, "Unauthorized");
}

---

🛠️ Implementation Details

1. AST Traversal & Detection Engine (rules/b004_ecrecover_zero.rs)

• AST Parsing: Leverages solang-parser to parse Solidity source units, inspecting functions, constructors, and modifiers.
• Variable Lifecycle & Assignment Tracking:
  • Detects Statement::VariableDefinition and Expression::Assign where ecrecover is invoked.
  • Identifies target variable names and inspects the scope for subsequent non-zero guards.
• Inline Invocation Inspection:
  • Traverses expression trees to identify inline ecrecover(...) calls.
  • Verifies whether the direct expression includes non-zero assertions (!= address(0), !(ecrecover == address(0))).
• Inline Assembly (Yul) Detection:
  • Inspects Statement::Assembly blocks for direct ecrecover opcode calls or staticcall(gas(), 1, ...) (precompile 1) lacking iszero / non-zero validations.
• Accepted Proof-of-Check Patterns:
  • require(signer != address(0), ...) / require(address(0) != signer, ...)
  • assert(signer != address(0))
  • if (signer == address(0)) revert CustomError();
  • Combined logical guards: require(signer != address(0) && signer == expected);
  • Safe library abstractions: OpenZeppelin's ECDSA.recover(hash, signature) or hash.recover(sig) (automatically recognized as safe and excluded from flags).

2. Actionable Remediation Guidance

Emits structured diagnostic findings with file, line number, Severity::High, and clear remediation instructions pointing to OpenZeppelin's battle-tested ECDSA library or explicit assertions.

---

🧪 Test Matrix & Fixtures (test/fixtures/b004_samples.sol)

A comprehensive test suite with 6 dedicated test cases and fixture evaluations:

Scenario / Function	Category	Expected Rule Result	Status
verifySignatureUnchecked	Variable assignment without zero check	🚨 Flagged (B004)	✅ PASS
executeAsAdmin	Direct inline require(ecrecover(...) == admin)	🚨 Flagged (B004)	✅ PASS
recoverSigner	Direct return ecrecover(...) without assertion	🚨 Flagged (B004)	✅ PASS
executeIfValidator	if (recovered == expected) without zero check	🚨 Flagged (B004)	✅ PASS
recoverAssembly	Yul precompile 1 call without iszero check	🚨 Flagged (B004)	✅ PASS
verifyGuardedRequire	Guarded with require(signer != address(0))	🟢 Clean / No Violation	✅ PASS
verifyCombinedRequire	Guarded with require(signer != address(0) && ...)	🟢 Clean / No Violation	✅ PASS
verifyGuardedRevert	Guarded with if (signer == address(0)) revert	🟢 Clean / No Violation	✅ PASS
verifyInlineGuarded	Guarded inline require(ecrecover != 0 && ...)	🟢 Clean / No Violation	✅ PASS
verifyECDSA	OpenZeppelin ECDSA.recover(hash, v, r, s)	🟢 Clean / No Violation	✅ PASS
verifyECDSAMember	OpenZeppelin hash.recover(signature)	🟢 Clean / No Violation	✅ PASS
setAdmin	Unrelated non-cryptographic logic	🟢 Clean / No Violation	✅ PASS

---

📂 File Changes Summary

File	Type	Description
rules/b004_ecrecover_zero.rs	✨ New	Full B004 static rule engine with AST visitor & unit tests
test/fixtures/b004_samples.sol	✨ New	Solidity test fixture with vulnerable and guarded patterns
rules/mod.rs	🔄 Modified	Export b004_ecrecover_zero module
rules/src/lib.rs	🔄 Modified	Register rule in rules library
rules/Cargo.toml	🔄 Modified	Update dependencies for serde serialization support

---

🚀 Verification Results

$ cargo test
running 9 tests
test rules::b004_ecrecover_zero::tests::invalid_solidity_returns_error ... ok
test rules::b004_ecrecover_zero::tests::openzeppelin_ecdsa_call_passes ... ok
test rules::b004_ecrecover_zero::tests::direct_comparison_without_zero_check_flagged ... ok
test rules::b004_ecrecover_zero::tests::guarded_with_revert_passes ... ok
test rules::b004_ecrecover_zero::tests::guarded_with_require_passes ... ok
test rules::b006_dead_storage::tests::test_public_getters_and_referenced_variables ... ok
test rules::b006_dead_storage::tests::test_shadowing_and_unreferenced_var ... ok
test rules::b006_dead_storage::tests::test_b006_samples_fixture ... ok
test rules::b004_ecrecover_zero::tests::fixture_parses_and_flags_all_vulnerabilities ... ok

test result: ok. 9 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

$ cargo test --manifest-path rules/Cargo.toml
test result: ok. 15 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

---

📋 Checklist

• Static rule B004 implemented in rules/b004_ecrecover_zero.rs
• Flags native ecrecover call expressions lacking non-zero address assertions
• Verifies variable assignments, inline expressions, and assembly precompiles
• Correctly identifies safe patterns (require != address(0), if == address(0) revert, OpenZeppelin ECDSA.recover)
• Fixture samples created under test/fixtures/b004_samples.sol
• Unit tests and fixture tests pass clean (cargo test)
• Changes committed to feature branch and pushed to remote