diff --git a/docs/tutorials/security-checklist-for-midnight-dapps-before-deployment.md b/docs/tutorials/security-checklist-for-midnight-dapps-before-deployment.md new file mode 100644 index 0000000..335a254 --- /dev/null +++ b/docs/tutorials/security-checklist-for-midnight-dapps-before-deployment.md @@ -0,0 +1,312 @@ +# Security Checklist for Midnight dApps Before Deployment + +## Overview + +Before deploying any Midnight smart contract to testnet or mainnet, run through this +security checklist. Most of the bugs below have caused real contract failures or +exploitable conditions in deployed dApps. Use it as the final gate before every +release. + +## What's Covered + +| Check | Risk if Missed | +|-------|---------------| +| Ownership verification | Anyone becomes owner on init | +| Arithmetic overflow | Under/overflow steals funds | +| Reentrancy | External call before state update → re-drain | +| Nullifier reuse | Spend fresh coins → proof fails or double-spend | +| Access control gaps | Public fn allows unauthorized state change | +| Initializer reentrancy | Proxy upgrade vulnerability | +| Pedersen commitment binding | Malicious proof uses wrong commitment | + +## Prerequisites + +- Compact smart contract written and compiling +- Midnight testnet running locally +- Understanding of `#[private]` and `#[public]` attributes + +## Architecture Note + +Midnight's L1 contract state is partially public. The ZK proof layer is **built by +the client**, not the contract. This means the security model differs from EVM Solidity: + +``` +┌─────────────────────────────────────────────────────────┐ +│ EVM Solidity Midnight Compact │ +│ - Contract on-chain stores all - L1 stores only │ +│ state deterministically public commitment │ +│ - All logic enforced by EVM + private state │ +│ - Reentrancy guards on-chain encrypted locally │ +│ - Access control = the gatekeeper → ZK proof must │ +│ be valid / valid │ +└─────────────────────────────────────────────────────────┘ +``` + +## Pre-Deployment Checklist + +Run ALL checks before deploying. Each failed item = block deployment. + +### 1. Ownership Verification + +``` +CLI: grep -n 'pub.*new()\|pub.*__init__\|owner' contracts/*.compact +``` + +**Pass criteria**: `owner` field exists and `#[private]`. Default `new()` sets it only once. + +```compact +struct MyContract { + #[private] + owner: Address, // ✅ Private — only the initializer knows it +} + +impl MyContract { + pub fn new(initial_owner: &Address) -> Self { + MyContract { owner: *initial_owner } // ✅ Set once, never changeable + } +} +``` + +**Failure mode**: No owner check, or `owner` is `pub`: +```compact +// ❌ ANY ADDRESS CAN SET THEMSELVES AS OWNER +struct BadContract { + owner: Address, // pub — anyone can see and override +} +``` + +### 2. Arithmetic Bounds + +``` +CLI: grep -n '+\|-\|\*' contracts/*.compact | grep -v '//' | head -30 +``` + +**Pass criteria**: All arithmetic has explicit bounds check or `checked_add/sub`. + +```compact +// ✅ Safe subtraction with bounds +pub fn safe_transfer(&self, recipient: &Address, amount: u64) -> bool { + let balance = self.balances.get(recipient); + if balance < amount { return false; } // Isolate failure + self.balances.set(recipient, balance - amount); + true +} +``` + +**Failure mode**: Direct subtract before check: +```compact +// ❌ Underflow wraps silently +let new_balance = old_balance - amount; +``` + +### 3. Reentrancy Guarding + +**Pass criteria**: All balance updates happen BEFORE any external L1 call +(emit, transfer, or note commitment). + +```compact +// ✅ SECURE — state update before external effects +pub fn withdraw(&mut self, amount: u64) -> bool { + let caller = self.caller(); + let balance = self.balances.get(&caller); + self.balances.set(&caller, balance - amount); // STATE FIRST + emit Withdrawn(caller, amount); // THEN external + true +} +``` + +**Failure mode** (reentrancy window): +```compact +// ❌ DANGEROUS — emit/transfer before state update +pub fn withdraw(&mut self, amount: u64) -> bool { + emit Withdrawn(caller, amount); + self.balances.set(&caller, balance - amount); // ← Reentrancy window open here +} +``` + +### 4. Nullifier and Commitment Hygiene + +``` +CLI: grep -n 'nullifier\|Nullifier\|commitment\|commit' contracts/*.compact +``` + +**Pass criteria**: +- No note is spent twice (nullifier uniqueness) +- Every `NullifierSpent` event has a corresponding valid `sendShielded` call +- Freshly committed notes cannot be spent in the same block + +```compact +// ✅ Nullifier tracked uniquely +let nullifier_field = some_blinding_field as Field; +self.spent_nullifiers.set(nullifier_field, true); +emit NullifierSpent(nullifier_field); // Emit BEFORE spending +``` + +### 5. #[private] Field Coverage + +``` +CLI: grep -n '^ *#\[private\]\|^ *#\[public\]' contracts/*.compact | grep -v 'tests/' +``` + +**Pass criteria**: All state fields that should be private are explicitly `#[private]`. +Default is public for struct fields — be explicit. + +```compact +record Note { + #[private] // ✅ Owner is encrypted on-chain + owner: Address, + #[private] // ✅ Value hidden in proof + value: u64, + #[public] // ✅ Commitment visible for Merkle + commitment: Field, +} +``` + +### 6. Initializer Idempotency + +``` +CLI: grep -n 'pub.*new\|pub.*__init__' contracts/*.compact +``` + +**Pass criteria**: Initializer can only be called once. Re-calling must panic. + +```compact +impl MyContract { + pub fn new(owner: &Address) -> Self { + assert(!is_initialized(), "Already initialized"); // ✅ Idempotent guard + MyContract { owner: *owner, initialized: true } + } +} +``` + +### 7. Pedersen Commitment Binding + +**Pass criteria**: Commitment input binds `owner + value + nonce` uniquely. +No malleable or predictable commitment. + +```compact +// ✅ Three inputs make commitment non-trivial to forge +let commitment = PedersenHasher::hash(vec![ + owner.to_field(), + value as Field, + nonce as Field, +]); +``` + +**Failure mode**: Commitment uses only 1 input (truncated): +```compact +// ❌ Predictable commitment structure +let commitment = PedersenHasher::hash(vec![value as Field]); // owner missing! +``` + +### 8. External Call Safety + +**Pass criteria**: No external calls (emit, transfer, callback) appear before state update. + +```compact +// ✅ SAFE ORDER — state mutates first, external events second +pub fn safe_fn(&mut self) { + self.balances.set(...); // 1. State + emit Event(...); // 2. External +} +``` + +### 9. Access Control Coverage + +``` +CLI: grep -n 'pub fn\|pub.*fn\|assert.*caller\|self.caller' contracts/*.compact +``` + +**Pass criteria**: State-changing functions check `self.caller()` is authorized. + +```compact +pub fn mint(&mut self, amount: u64) { + let caller = self.caller(); + assert(caller == self.owner, "Only owner can mint"); // ✅ + ... +} +``` + +### 10. Merkle Tree Finality + +**Pass criteria**: Contract does not assume Merkle root is final in the same block +as a commitment emission. Wait ≥1 L1 confirmation before spending newly minted coins. + +## Automated Check Script + +Save as `scripts/security-check.sh`: + +```bash +#!/usr/bin/env bash +set -e +CONTRACT=${1:-contracts/main.compact} + +echo "=== Midnight dApp Security Check ===" +echo "" + +# 1. Owner check +echo "1. Ownership field:" +grep -n 'owner' "$CONTRACT" | grep -v '^Binary' | head -5 + +# 2. #[private] coverage +echo "2. Private field coverage:" +grep -E '#\[(private|public)\]' "$CONTRACT" + +# 3. Arithmetic safety +echo "3. Arithmetic operations:" +grep -E '\+|\-|\*' "$CONTRACT" | grep -v '^Binary' | head -10 + +# 4. Reentrancy check +echo "4. External call ordering:" +grep -n 'emit\|send\|transfer\|commit' "$CONTRACT" | head -10 + +# 5. Nullifier hygiene +echo "5. Nullifier/commitment events:" +grep -n 'Nullifier\|nul\|commit' "$CONTRACT" | head -10 + +# 6. Initializer guard +echo "6. Initializer signature:" +grep -n 'pub fn new\|pub fn __init__' "$CONTRACT" + +echo "" +echo "Review completed. Verify each item above meets pass criteria." +``` + +Run before every deployment: +```bash +bash scripts/security-check.sh contracts/my_contract.compact +``` + +## Summary Checklist + +``` +CLI Checks: +[ ] grep -n 'pub.*new()' → Owner set deterministically +[ ] grep -E '^ *#\[private\]' → Sensitive fields explicit +[ ] grep -n 'Nullifier\|commit' → Nullifier events present +[ ] grep -n 'emit\|send' → External calls only after state + +Manual Checks: +[ ] Owner = #[private] field, initialized once +[ ] All arithmetic = checked_add/sub or explicit bounds check +[ ] Balance updates BEFORE emit/transfer calls +[ ] No note can be spent twice (nullifier tracking) +[ ] Every shielded spend has NullifierSpent event +[ ] Fresh notes = wait 1 L1 confirmation before spend +[ ] Assertions guard every state-changing fn +[ ] Merkle root race condition documented / handled + +Result: +□ PASS → Deploy +□ FAIL → Fix item, re-run script, return here +``` + +## Common Issues: Quick Reference + +| Symptom | Diagnosis | Fix | +|---------|-----------|-----| +| "Double spend" reorg | Nullifier already spent | Check NullifierSpent events; never reuse | +| Deploy fails with owner = null | `new()` not setting field | Add `owner: initial_owner` in struct init | +| Silent arithmetic bug | Subtraction wraps silently | Use `checked_sub` or `if balance < amount { return false }` | +| Frontend calls private fn | Contract exposes pub fn no guard | Add `assert(caller == owner)` | +| Merkle root mismatch at spend | Indexer not updated yet | Wait 1 confirmation or use `mint_and_send` |