72 lines
5.4 KiB
Markdown
72 lines
5.4 KiB
Markdown
# MEV Bot Security Audit
|
|
**Date:** 2025-11-03
|
|
**Auditor:** Codex (GPT-5)
|
|
|
|
## 1. Scope & Methodology
|
|
- Reviewed on-chain contracts in `contracts/` with emphasis on `ProductionArbitrageExecutor.sol` and Balancer flash-loan receivers.
|
|
- Inspected Go runtime components under `pkg/security`, `pkg/arbitrum`, `pkg/transport`, and CLI tooling in `cmd/`.
|
|
- Assessed operational scripts and configuration artifacts within `scripts/`, `config/`, and documentation references.
|
|
- Static analysis only; no live transactions executed.
|
|
|
|
## 2. Executive Summary
|
|
The repository still embeds production credentials (Chainstack RPC token and an Alchemy API key), contains placeholder authentication logic, and ships a non-compilable main arbitrage contract. These issues allow credential theft, brute-force attacks on wallet material, and prevent reliable deployment. Immediate remediation is required before any production use.
|
|
|
|
## 3. Critical Findings
|
|
|
|
### C-01 Hardcoded RPC Credential
|
|
- **Location:** `pkg/arbitrum/connection.go:197`, `pkg/pools/discovery.go:594-597`, `pkg/market/manager.go:118-123`, `README.md:68-69`
|
|
- **Risk:** Embedded Chainstack token grants public RPC access; attackers can hijack traffic or exhaust quotas. The client falls back to this token silently, so operators may unintentionally deploy with leaked credentials.
|
|
- **Recommendation:** Rotate the Chainstack token. Remove every hardcoded endpoint, require operators to supply credentials via secrets storage or environment variables, and add CI checks that block `chainstack.com/<token>` strings.
|
|
|
|
### C-02 Exposed Alchemy API Key
|
|
- **Location:** `scripts/check-wallet-balance.sh:8`
|
|
- **Risk:** Public key can be abused for unlimited balance queries, risking rate-limit bans or service suspension.
|
|
- **Recommendation:** Treat the key as compromised, rotate it, and load RPC URLs from configuration rather than shipping them in scripts.
|
|
|
|
### C-03 Placeholder Authentication With Static Passwords
|
|
- **Location:** `pkg/security/keymanager.go:1550-1567`
|
|
- **Risk:** Any adversary can authenticate with `secure_admin_password_123` or `default_password`, granting access to signing keys.
|
|
- **Recommendation:** Remove the stub entirely. Integrate a real credential store (bcrypt/argon2 hashes, salted) and enforce MFA/whitelisting policies defined in config.
|
|
|
|
### C-04 Weak Keystore Derivation
|
|
- **Location:** `pkg/security/keymanager.go:295-301`
|
|
- **Risk:** Uses `keystore.LightScryptN/ LightScryptP` which is labelled “testing only” in go-ethereum and reduces brute-force cost dramatically.
|
|
- **Recommendation:** Restore `keystore.StandardScryptN`/`StandardScryptP` for production or migrate to Argon2id. Update tests/benchmarks accordingly.
|
|
|
|
### C-05 Unsafe Legacy Flash-Loan Executor
|
|
- **Location:** `contracts/balancer/FlashLoanReceiver.sol:112-139`
|
|
- **Risk:** Sets `amountOutMinimum` to zero and leaves token approvals unlimited, enabling routers to drain assets if trusted routes are compromised.
|
|
- **Recommendation:** Retire this contract or align it with the hardened `FlashLoanReceiverSecure` implementation (bounded slippage, allowance resets, SafeERC20).
|
|
|
|
### C-06 Non-Compilable Production Arbitrage Contract
|
|
- **Location:** `contracts/ProductionArbitrageExecutor.sol:548-570`
|
|
- **Risk:** Relies on `getRoleMember` without importing `AccessControlEnumerable`, so the supposedly “production-ready” contract cannot deploy. Documentation claiming the bytecode is live is inaccurate.
|
|
- **Recommendation:** Either extend `AccessControlEnumerable` or redesign withdrawals to avoid enumeration. Recompile and update docs/tests after the fix.
|
|
|
|
## 4. Medium Findings
|
|
|
|
### M-01 Insecure Private-Key Handling in Scripts
|
|
- **Location:** `scripts/check-wallet-balance.sh:7`, `scripts/setup-keystore.sh:8`
|
|
- **Risk:** Reads secrets from `/tmp/wallet_key.txt`, a predictable world-readable path on many systems.
|
|
- **Recommendation:** Require user-supplied secure paths or interactive input. Wipe temporary buffers after use.
|
|
|
|
### M-02 Default Test Private Key Fallback
|
|
- **Location:** `scripts/deploy-pool-detector.sh:30-37`
|
|
- **Risk:** Deployment scripts silently use a known Foundry test key, risking accidental mainnet deployment with a public private key.
|
|
- **Recommendation:** Fail fast if `PRIVATE_KEY` is unset. Keep test keys in local-only templates.
|
|
|
|
## 5. Positive Observations
|
|
- `contracts/balancer/FlashLoanReceiverSecure.sol` addresses the audit findings (slippage bounds, SafeERC20, non-reentrancy); prioritize this contract for deployment.
|
|
- `internal/logger` employs structured logging with secret scrubbing and level-based filtering, which will aid operational monitoring once credentials are externalized.
|
|
|
|
## 6. Recommendations & Next Steps
|
|
1. Rotate all leaked RPC/API credentials immediately and scrub them from repository history.
|
|
2. Replace the placeholder authentication/KDF logic with production-grade implementations and add regression tests.
|
|
3. Migrate to `FlashLoanReceiverSecure` and ensure `ProductionArbitrageExecutor` compiles; rerun Foundry and Go test suites post-changes.
|
|
4. Harden operational scripts to avoid writing secrets to predictable locations and to fail without explicit credentials.
|
|
5. Introduce CI checks (e.g., `ripgrep` rules) to block committed secrets, Chainstack/Alchemy tokens, or default passwords going forward.
|
|
|
|
## 7. Testing Performed
|
|
- Manual static review; no automated tests were executed in this pass. Re-run `go test ./...`, Foundry unit/integration tests, and any existing CI workflows after remediation.
|
|
|