Add skill: auditing-foundry-smart-contract-security

Pre-deployment security audit skill for Solidity contracts in Foundry projects. Complements analyzing-ethereum-smart-contract-vulnerabilities (which it is based on) with a dev-side, Foundry-first workflow and full key-hygiene coverage. Layers four independent techniques: - Static analysis: Slither (90+ detectors) + Aderyn (Cyfrin) - Symbolic execution: Mythril (optional) - Property-based testing: forge fuzz + invariant tests (handler pattern) - Manual review checklist + secrets/keystore audit Includes scripts/agent.py (orchestrator aggregating Slither/Aderyn/Mythril/forge test + coverage + private-key scan into a JSON report with a PASS/FAIL deploy gate) and three references (tool cheat-sheets, SWC vulnerability checklist, secure deployment & key hygiene with cast keystore / multisig). Passes tools/validate-skill.py. Slither, Aderyn, forge test/coverage parsing and the gate logic were verified end-to-end against a reentrancy-vulnerable contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-26 22:20:59 +03:00 · 2026-06-16 15:52:33 +02:00
parent 04450304b1
commit 25e0bc60e8
6 changed files with 1039 additions and 0 deletions
@@ -0,0 +1,181 @@
+# API Reference — Tooling
+
+## Slither (static analysis)
+
+```bash
+slither .                                  # whole project (reads foundry.toml + remappings)
+slither . --json slither-report.json       # machine-readable
+slither . --json -                          # JSON to stdout (used by agent.py)
+slither . --print human-summary             # quick overview
+slither . --print inheritance-graph         # inheritance / proxy layout
+slither . --detect reentrancy-eth,unprotected-upgrade   # specific detectors
+slither --list-detectors                    # all 90+ detectors
+slither . --exclude-informational --exclude-low          # focus high/medium
+slither . --triage-mode                     # interactively suppress false positives -> slither.db.json
+```
+
+Severity matrix (impact × confidence):
+
+| Impact | Confidence | Example detectors |
+|--------|------------|-------------------|
+| High | High | `reentrancy-eth`, `suicidal`, `arbitrary-send-eth` |
+| High | Medium | `controlled-delegatecall`, `reentrancy-no-eth` |
+| Medium | High | `locked-ether`, `incorrect-equality`, `tx-origin` |
+| Medium | Medium | `uninitialized-state`, `shadowing-state`, `unchecked-transfer` |
+| Low | High | `naming-convention`, `solc-version`, `low-level-calls` |
+| Informational | High | `pragma`, `dead-code`, `assembly` |
+
+## Aderyn (Cyfrin, Rust static analyzer — complementary to Slither)
+
+```bash
+aderyn .                                    # markdown report.md by default
+aderyn . -o aderyn-report.json              # JSON (used by agent.py)
+aderyn . --scope src/                       # limit scope
+```
+
+## Mythril (symbolic execution — slow, use on critical contracts only)
+
+```bash
+myth analyze src/Vault.sol -o json
+myth analyze src/Vault.sol --execution-timeout 300 --max-depth 50 -o json
+myth analyze --address 0x... --rpc <url>    # deployed bytecode (read-only)
+```
+
+## Foundry — testing
+
+```bash
+forge build                                 # required before static analysis
+forge test -vvv                             # unit + fuzz; -vvvv shows traces
+forge test --match-contract VaultTest
+forge test --match-test invariant_          # invariant suite only
+forge coverage --report summary             # line/branch coverage table
+forge coverage --report lcov                # for CI / tooling
+forge snapshot                              # gas snapshots (DoS-by-gas review)
+forge fmt --check                           # style gate
+```
+
+### Fuzz test (property over random inputs)
+
+```solidity
+function testFuzz_SetNumber(uint256 x) public {
+    counter.setNumber(x);
+    assertEq(counter.number(), x);
+}
+```
+
+### Revert test (replaces deprecated testFail)
+
+```solidity
+function test_RevertWhen_Unauthorized() public {
+    vm.prank(attacker);
+    vm.expectRevert("Not authorized");   // or vm.expectRevert(MyError.selector)
+    target.adminOnly();
+}
+```
+
+### Key cheatcodes (`vm.*`)
+
+| Cheatcode | Use |
+|-----------|-----|
+| `vm.prank(addr)` / `vm.startPrank` | impersonate caller (test access control) |
+| `vm.warp(ts)` / `vm.roll(n)` | manipulate `block.timestamp` / `block.number` |
+| `vm.deal(addr, amt)` | set ETH balance |
+| `vm.store(addr, slot, val)` | overwrite storage (test invariants under hostile state) |
+| `vm.expectRevert(...)` | assert a call reverts (with msg / custom error selector) |
+| `vm.expectEmit(...)` | assert events |
+| `bound(x, lo, hi)` | constrain fuzz inputs in handlers |
+| `makeAddr("name")` | deterministic labelled actor |
+
+### Invariant testing — handler pattern (the important one)
+
+A handler wraps the target, **bounds inputs**, rotates **actors**, and tracks
+**ghost variables**; `targetContract(handler)` makes the fuzzer drive only the
+handler so sequences stay realistic.
+
+```solidity
+// test/Invariant.t.sol
+contract VaultInvariantTest is Test {
+    Vault vault;
+    VaultHandler handler;
+
+    function setUp() public {
+        vault = new Vault();
+        handler = new VaultHandler(vault);
+        targetContract(address(handler));         // fuzz the handler, not the vault directly
+    }
+
+    function invariant_ConservationOfDeposits() public view {
+        assertEq(address(vault).balance,
+                 handler.ghost_depositSum() - handler.ghost_withdrawSum());
+    }
+    function invariant_Solvency() public view {
+        assertGe(address(vault).balance, vault.totalDeposits());
+    }
+}
+```
+
+```solidity
+// test/handlers/VaultHandler.sol
+contract VaultHandler is Test {
+    Vault public vault;
+    uint256 public ghost_depositSum;
+    uint256 public ghost_withdrawSum;
+    address[] public actors;
+    address internal currentActor;
+
+    modifier useActor(uint256 seed) {
+        currentActor = actors[bound(seed, 0, actors.length - 1)];
+        vm.startPrank(currentActor); _; vm.stopPrank();
+    }
+    constructor(Vault _v) {
+        vault = _v;
+        for (uint256 i; i < 10; i++) { actors.push(makeAddr(string(abi.encodePacked("actor", i)))); vm.deal(actors[i], 100 ether); }
+    }
+    function deposit(uint256 amt, uint256 seed) external useActor(seed) {
+        amt = bound(amt, 0.01 ether, 10 ether);
+        vault.deposit{value: amt}(); ghost_depositSum += amt;
+    }
+    function withdraw(uint256 amt, uint256 seed) external useActor(seed) {
+        uint256 bal = vault.balanceOf(currentActor);
+        if (bal == 0) return;
+        amt = bound(amt, 1, bal);
+        vault.withdraw(amt); ghost_withdrawSum += amt;
+    }
+}
+```
+
+Tune in `foundry.toml`:
+
+```toml
+[invariant]
+runs = 256
+depth = 128
+fail_on_revert = false   # set true once the handler fully constrains inputs
+
+[fuzz]
+runs = 10000
+```
+
+## SWC Registry (key entries)
+
+| SWC | Title | Detected by |
+|-----|-------|-------------|
+| SWC-101 | Integer Overflow/Underflow | Mythril (pre-0.8 only) |
+| SWC-104 | Unchecked Call Return | Slither + Mythril |
+| SWC-105 | Unprotected Ether Withdrawal | Slither + Mythril |
+| SWC-106 | Unprotected SELFDESTRUCT | Slither + Mythril |
+| SWC-107 | Reentrancy | Slither + Mythril |
+| SWC-112 | Delegatecall to Untrusted Callee | Slither |
+| SWC-114 | Transaction Order Dependence (front-running) | manual |
+| SWC-115 | tx.origin Authentication | Slither |
+| SWC-116 | Block Timestamp Dependence | Mythril |
+| SWC-120 | Weak Randomness | Slither + manual |
+
+## References
+- Slither: https://github.com/crytic/slither
+- Aderyn: https://github.com/Cyfrin/aderyn
+- Mythril: https://github.com/Consensys/mythril
+- Foundry Book: https://getfoundry.sh/
+- SWC Registry: https://swcregistry.io/
+- Solidity security: https://docs.soliditylang.org/en/latest/security-considerations.html
+- Solodit (audit findings DB): https://solodit.xyz/
@@ -0,0 +1,90 @@
+# Secure Deployment & Key Hygiene
+
+The contract code can be flawless and you still lose everything if a **private key
+leaks** or you sign a malicious transaction. This is the part most smart-contract
+guides skip. Treat keys as the highest-severity asset.
+
+## Golden rules
+
+1. **A real private key or seed phrase NEVER touches a file, env var, shell history, or git.**
+2. Plaintext `PRIVATE_KEY=0x...` in `.env` is the #1 leak vector — use an **encrypted keystore** instead.
+3. **Separate wallets**: a throwaway dev wallet (testnet only) ≠ the mainnet deployer ≠ your personal MetaMask with real funds.
+4. **Hardware wallet (Ledger/Trezor) for any mainnet deploy or admin action** that controls funds.
+5. Simulate before broadcasting; verify after.
+
+## Foundry encrypted keystore (`cast wallet`)
+
+Import the key once into an encrypted, password-protected keystore — then reference
+it by name. The raw key never appears in commands or files again.
+
+```bash
+# Import interactively (key is typed, not in argv/history), set a strong password
+cast wallet import deployer --interactive
+
+# Or generate a fresh dev key directly into the keystore
+cast wallet new
+
+# List / inspect (addresses only)
+cast wallet list
+```
+
+Deploy by **account name**, never by `--private-key`:
+
+```bash
+# Testnet first — simulate (no --broadcast) then broadcast + verify
+forge script script/Deploy.s.sol --account deployer --rpc-url <testnet_rpc>
+forge script script/Deploy.s.sol --account deployer --rpc-url <testnet_rpc> --broadcast --verify
+
+# Mainnet (prefer a Ledger):
+forge script script/Deploy.s.sol --ledger --hd-paths "m/44'/60'/0'/0/0" --rpc-url <mainnet_rpc> --broadcast --verify
+```
+
+In deploy scripts, use `vm.startBroadcast()` with **no argument** (it uses the
+`--account`/`--ledger` signer). Avoid `vm.envUint("PRIVATE_KEY")`.
+
+## Anti-leak controls (wire into the project)
+
+```bash
+# 1. .gitignore the usual suspects
+printf '.env\n.env.*\n*.key\nkeystore/\nbroadcast/\n' >> .gitignore
+
+# 2. Scan history + working tree for secrets (see the implementing-secret-scanning-with-gitleaks skill)
+gitleaks detect --no-banner
+gitleaks detect --no-banner --log-opts="--all"   # full git history
+
+# 3. Confirm nothing sensitive is tracked
+git ls-files | grep -E '\.env$|\.key$|keystore' && echo "REMOVE THESE FROM GIT"
+```
+
+If a key was ever committed (even and then deleted): **consider it compromised** —
+generate a new one, move funds, and purge history (BFG / `git filter-repo`).
+
+## MetaMask / wallet operational security
+
+- Dedicated browser profile for Web3; review every signature — **read what you sign**.
+- Beware **blind signing** and `eth_sign`/`personal_sign` phishing; reject opaque hex.
+- Token **approval hygiene**: avoid unlimited `approve`; periodically revoke (revoke.cash); prefer `permit` with deadlines.
+- Verify the **contract address and chain id** before interacting; bookmark dApps, don't follow links.
+- Add networks/RPCs only from trusted sources — a malicious RPC can lie about state and simulate fake balances.
+
+## RPC & dependency trust
+
+- Pin a reputable RPC (your own node, or a known provider); a hostile RPC can feed false data to scripts and frontends.
+- Pin dependency versions (`forge install` with a tag/commit; lock OpenZeppelin version). Re-audit on bumps.
+- Verify deployed bytecode matches source on the explorer (`forge verify-contract` / `--verify`).
+
+## Post-deploy checklist
+
+- [ ] Source verified on the block explorer.
+- [ ] Ownership/admin transferred to a **multisig** (Safe), not an EOA, for anything controlling funds.
+- [ ] Timelock on privileged upgrades/parameter changes.
+- [ ] Monitoring/alerting on critical events (large withdrawals, ownership changes, pause).
+- [ ] Emergency runbook: pause + emergency-withdraw path tested on testnet.
+- [ ] Deploy key rotated/retired if it ever touched a less-trusted machine.
+
+## References
+- Foundry deploying guide: https://getfoundry.sh/guides/deploying
+- `cast wallet`: https://getfoundry.sh/cast/reference/wallet
+- OpenZeppelin Contracts: https://docs.openzeppelin.com/contracts
+- Safe (multisig): https://safe.global/
+- revoke.cash (approval management): https://revoke.cash/
@@ -0,0 +1,78 @@
+# Manual Review Checklist — Solidity / EVM
+
+Walk this for **every** contract that moves value. Each item: what to look for →
+how to confirm (Foundry test / Slither detector) → fix. Tools catch the known
+patterns; this catches the logic bugs they can't.
+
+## 1. Reentrancy (SWC-107)
+- [ ] External call (`call`, `transfer`, ERC-777 hooks, ERC-721 `onERC...Received`, arbitrary token) **before** state updates?
+- [ ] Cross-function / read-only reentrancy: a view used by another protocol mid-call?
+- **Confirm:** Slither `reentrancy-eth`/`reentrancy-no-eth`; write an attacker contract test that re-enters in its `receive()`.
+- **Fix:** Checks-Effects-Interactions order; `nonReentrant` (OpenZeppelin `ReentrancyGuard`); pull-over-push payments. ("OZ" = OpenZeppelin throughout.)
+
+## 2. Access control (SWC-105/106/115)
+- [ ] Every state-changing/admin function gated (`onlyOwner`, roles, custom modifier)?
+- [ ] `initialize()` on upgradeable contracts protected against re-init and front-running?
+- [ ] No `tx.origin` for auth (phishable) — use `msg.sender`.
+- [ ] `selfdestruct` / `delegatecall` reachable only by trusted roles?
+- **Confirm:** a `test_RevertWhen_*` with `vm.prank(attacker)` + `vm.expectRevert` for each guard.
+- **Fix:** OZ `Ownable2Step` / `AccessControl`; `initializer` modifier; remove `tx.origin`.
+
+## 3. Oracle / price manipulation (DeFi #1 exploit class)
+- [ ] Price from spot `getReserves()` / a single AMM pool? (flash-loan manipulable)
+- [ ] Using Chainlink: checked `updatedAt` staleness, `answeredInRound`, min/max bounds, L2 sequencer uptime?
+- **Confirm:** fork test that manipulates the pool within one tx and asserts your protocol stays solvent.
+- **Fix:** TWAP / Chainlink with staleness+deviation checks; never trust spot for pricing.
+
+## 4. Arithmetic & rounding (SWC-101)
+- [ ] Solidity ^0.8 (built-in checked math) — and any `unchecked{}` block justified?
+- [ ] Division before multiplication (precision loss)? Rounding always in protocol's favor?
+- [ ] Share-inflation / first-depositor attack on ERC-4626-style vaults?
+- **Confirm:** `testFuzz_*` over amounts; invariant `assertGe(assets, shares-implied)`.
+- **Fix:** mulDiv (OZ `Math.mulDiv`); virtual shares/offset for vaults; explicit rounding direction.
+
+## 5. Unchecked external calls / return values (SWC-104)
+- [ ] Return value of low-level `call`/`send` and ERC-20 `transfer`/`transferFrom` checked?
+- [ ] Non-standard ERC-20s (no return, fee-on-transfer, rebasing) handled?
+- **Confirm:** Slither `unchecked-transfer`, `unchecked-lowlevel`.
+- **Fix:** OZ `SafeERC20`; measure balance delta for fee-on-transfer; require success.
+
+## 6. delegatecall / proxy storage (SWC-112)
+- [ ] Storage layout identical across implementation upgrades (no reordered/removed vars)?
+- [ ] No `delegatecall` to user-supplied address; implementation can't be re-initialized/self-destructed?
+- **Confirm:** Slither `controlled-delegatecall`, `unprotected-upgrade`; storage-layout diff between versions.
+- **Fix:** OZ `UUPS`/`Transparent` proxies + storage gaps; `_disableInitializers()` in constructor.
+
+## 7. Front-running / MEV / ordering (SWC-114)
+- [ ] Approve race (ERC-20 `approve` from non-zero to non-zero)?
+- [ ] Slippage / deadline params on swaps and mints? Commit-reveal where needed?
+- **Fix:** `increaseAllowance`/`permit`; enforce `minOut` + `deadline`; commit-reveal for sensitive ordering.
+
+## 8. Randomness (SWC-120)
+- [ ] Any `block.timestamp`/`blockhash`/`prevrandao` used as randomness for value?
+- **Fix:** Chainlink VRF; never on-chain pseudo-randomness for payouts.
+
+## 9. Denial of service (SWC-113/128)
+- [ ] Unbounded loops over user-growable arrays? Push payments that can revert and brick the contract?
+- [ ] Single external dependency whose revert blocks all users?
+- **Fix:** pull payments; bounded iteration / pagination; isolate failures.
+
+## 10. Token / standard pitfalls
+- [ ] ERC-20: decimals assumptions, fee-on-transfer, rebasing, missing return.
+- [ ] ERC-721/1155: safe-transfer reentrancy hooks; approval scope.
+- [ ] Permit (EIP-2612): replay, deadline, signature malleability.
+
+## 11. General hygiene
+- [ ] `pragma` pinned (`pragma solidity 0.8.26;` not `^`)? Latest stable solc?
+- [ ] Events emitted for every state-changing action (off-chain monitoring)?
+- [ ] No leftover `console.log` / test backdoors / hardcoded addresses?
+- [ ] Pausability + emergency withdraw for value-holding contracts?
+- [ ] Invariants written for every conservation law (total supply, solvency, accounting)?
+
+## Severity triage (impact × likelihood)
+- **Critical/High** → direct loss/lock of funds, unauthorized mint/withdraw, broken access control. **Blocks deploy.**
+- **Medium** → loss under specific conditions, griefing, precision drift.
+- **Low/Info** → best-practice, gas, style.
+
+Cross-check real-world findings on **Solodit** (https://solodit.xyz/) for the contract type
+you're shipping (vault, AMM, staking, bridge, governance).