diff --git a/.mise.toml b/.mise.toml index 5f739db..763f5b5 100644 --- a/.mise.toml +++ b/.mise.toml @@ -85,8 +85,8 @@ run = [ [tasks.ci-fast] description = "Fast CI checks before push to feat/*" -timeout = "180s" -run = "pnpm typecheck && pnpm lint && uvx semgrep scan --config=auto --config=./.semgrep.yml --error && cd contracts && forge test --no-match-test invariant && cd ../circuits && pnpm test" +timeout = "600s" +run = "pnpm typecheck && pnpm lint && uvx semgrep scan --config=auto --config=./.semgrep.yml --severity ERROR --error && cd contracts && forge test --no-match-test invariant && cd ../circuits && pnpm test:fast" [tasks.ci-full] description = "Full CI before PR to dev/main" @@ -117,7 +117,7 @@ description = "Run Slither static analysis" run = "cd contracts && uv run slither ." [tasks.semgrep] -description = "Run Semgrep with MARK custom rules" +description = "Run Semgrep security scan (contracts + circuits)" run = "uvx semgrep scan --config=auto --config=p/security-audit --config=./.semgrep.yml --error" [tasks.clean] diff --git a/= b/= deleted file mode 100644 index e69de29..0000000 diff --git a/circuits/scripts/test-fast.mjs b/circuits/scripts/test-fast.mjs deleted file mode 100644 index 4f7f80a..0000000 --- a/circuits/scripts/test-fast.mjs +++ /dev/null @@ -1,22 +0,0 @@ -// CI-fast witness tests: reuse existing circom build when present; otherwise build once. -import { existsSync } from "fs"; -import { spawnSync } from "child_process"; -import path from "path"; -import { fileURLToPath } from "url"; - -const root = path.join(path.dirname(fileURLToPath(import.meta.url)), ".."); -const wasm = path.join(root, "build/MARKPool_js/MARKPool.wasm"); - -function run(cmd, args) { - // nosemgrep:security.detect-child-process.detect-child-process - // Safe: only called with hardcoded 'pnpm'/'node' and controlled args - const r = spawnSync(cmd, args, { cwd: root, stdio: "inherit", shell: false }); - if (r.status !== 0) process.exit(r.status ?? 1); -} - -if (!existsSync(wasm)) { - console.log("circuits test:fast — no wasm artifact; running build once"); - run("pnpm", ["run", "build"]); -} - -run("node", ["test/MARKPool.test.mjs"]); diff --git a/circuits/test/MARKPool.fast.mjs b/circuits/test/MARKPool.fast.mjs index e30edcb..9ddba82 100644 --- a/circuits/test/MARKPool.fast.mjs +++ b/circuits/test/MARKPool.fast.mjs @@ -72,6 +72,21 @@ const DOMAIN_NULLIFIER_TAG = DOMAIN_VERSION * 100n + DOMAIN_NULLIFIER; const DEPTH = 20; const CHAIN_ID = 11155420n; // OP Sepolia +const RELAYER_MAX = 2n ** 160n; +// BN254 scalar field prime (matches circom's field) +const BN254_P = 21888242871839275222246405745257275088548364400416034343698204186575808495617n; + +// Modular inverse via extended Euclidean algorithm +function modInverse(a, m) { + let [old_r, r] = [a % m, m]; + let [old_s, s] = [1n, 0n]; + while (r !== 0n) { + const q = old_r / r; + [old_r, r] = [r, old_r - q * r]; + [old_s, s] = [s, old_s - q * s]; + } + return ((old_s % m) + m) % m; +} // Build a valid note function makeNote(amount, secret, blinding) { @@ -154,6 +169,7 @@ const validBase = { withdrawOwner: 0n, withdrawRecipient: 0n, withdrawAmount: 0n, + inSecret0Upper: 0n, }; console.log("MARKPool circuit fast tests"); @@ -161,6 +177,77 @@ console.log("MARKPool circuit fast tests"); // Happy path - core functionality await expectPass("valid 2-in 2-out transact", validBase); +// Withdrawal path - exercises inSecret0Upper binding constraint +const withdrawOwner = in0.secret % RELAYER_MAX; // lower 160 bits +const withdrawBase = { + ...validBase, + fee: 300n, + withdrawOwner, + withdrawRecipient: 0x70997970C51812dc3A010C7d01b50e0d17dc79C8n, + withdrawAmount: 200n, + inSecret0Upper: in0.secret / RELAYER_MAX, +}; +await expectPass("valid 2-in 2-out transact with withdrawal", withdrawBase); + +// Happy path with non-zero inSecret0Upper: secret = RELAYER_MAX + 42n so upper = 1n, lower = 42n. +// This exercises the `inSecret0Upper * 2^160` term in the binding constraint. +const in0Large = makeNote(500n, RELAYER_MAX + 42n, 999n); +const treeLarge = buildTwoLeafRoot(in0Large.commitment, in1.commitment, DEPTH); +const nullifier0Large = makeNullifier(in0Large, CHAIN_ID); +await expectPass("withdrawal with non-zero inSecret0Upper (upper-bits binding)", { + inAmount: [in0Large.amount, in1.amount], + inSecret: [in0Large.secret, in1.secret], + inBlinding: [in0Large.blinding, in1.blinding], + inPathElements: [treeLarge.path0.elements, treeLarge.path1.elements], + inPathIndices: [treeLarge.path0.indices, treeLarge.path1.indices], + outAmount: [out0Amount, out1Amount], + outSecret: [out0Secret, out1Secret], + outBlinding: [out0Blinding, out1Blinding], + merkleRoot: treeLarge.root, + chainId: CHAIN_ID, + dstChainId: CHAIN_ID, + protocolEpoch: 0n, + fee: 300n, + relayer: 0n, + nullifier: [nullifier0Large, nullifier1], + outCommitment: [outC0, outC1], + withdrawOwner: in0Large.secret % RELAYER_MAX, // 42n — lower 160 bits + withdrawRecipient: 0x70997970C51812dc3A010C7d01b50e0d17dc79C8n, + withdrawAmount: 200n, + inSecret0Upper: in0Large.secret / RELAYER_MAX, // 1n — non-zero upper bits +}); + +await expectFail("wrong withdrawOwner (tampered)", { + ...withdrawBase, + withdrawOwner: 222n, +}); +await expectFail("withdraw amount non-zero but owner zero", { + ...withdrawBase, + withdrawOwner: 0n, +}); +await expectFail("withdraw amount non-zero but recipient zero", { + ...withdrawBase, + withdrawRecipient: 0n, +}); +await expectFail("withdraw amount zero but owner non-zero", { + ...validBase, + withdrawOwner: withdrawOwner, + withdrawRecipient: 0n, + withdrawAmount: 0n, +}); + +// Forged-upper attack: prover supplies inSecret0Upper = (secret - spoofedOwner) * RELAYER_MAX^-1 mod p +// This satisfies the linear decomposition equation in-field but inSecret0Upper is ~254 bits, +// which the Num2Bits(93) range check must reject. +const spoofedOwner = 222n; +const forgedUpper = + ((in0.secret - spoofedOwner + BN254_P) % BN254_P) * modInverse(RELAYER_MAX, BN254_P) % BN254_P; +await expectFail("forged-upper owner binding bypass", { + ...withdrawBase, + withdrawOwner: spoofedOwner, + inSecret0Upper: forgedUpper, +}); + // Balance equation - critical invariants await expectFail("fee too low (balance broken)", { ...validBase, fee: fee - 1n }); await expectFail("fee too high (balance broken)", { ...validBase, fee: fee + 1n }); @@ -185,5 +272,9 @@ await expectFail("zero input amount", { inAmount: [0n, in1.amount], fee: in1.amount, }); +await expectFail("wrong output commitment", { + ...validBase, + outCommitment: [outC0 + 1n, outC1], +}); console.log("\nAll fast tests passed.");