diff --git a/.eslinttsconfigrc b/.eslinttsconfigrc index 0f5ac5170..c459504aa 100644 --- a/.eslinttsconfigrc +++ b/.eslinttsconfigrc @@ -1,4 +1,4 @@ { "extends": "./tsconfig.json", - "include": ["./typechain", "./deploy", "./tests", "./script", "./scenario", "saddle.config.js", "docgen-templates", "commitlint.config.js", "./hardhat.config.zksync.ts", "type-extensions.ts"] + "include": ["./typechain", "./deploy", "./tests", "./script", "./scripts", "./scenario", "saddle.config.js", "docgen-templates", "commitlint.config.js", "./hardhat.config.zksync.ts", "type-extensions.ts"] } diff --git a/.gitignore b/.gitignore index 80fbc3543..f19c782e8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,9 @@ tests/Scenarios allFiredEvents +# Foundry (XVSVault fuzz layer) — build artifacts + vendored forge-std +out-forge/ +cache-forge/ +lib/forge-std/ .build-temp build build_ diff --git a/foundry.toml b/foundry.toml new file mode 100644 index 000000000..b250cb209 --- /dev/null +++ b/foundry.toml @@ -0,0 +1,45 @@ +# Foundry layer for XVSVault invariant/fuzz testing. +# Isolated from the Hardhat setup: separate out/cache dirs, sources scoped to +# test-fuzz/ only (the 0.5.16 anchor pulls in just the XVSVault graph, not the +# whole protocol). Hardhat's artifacts/ and cache/ are untouched. +[profile.default] +src = "test-fuzz" +test = "test-fuzz" +libs = ["lib"] +out = "out-forge" +cache_path = "cache-forge" +auto_detect_solc = true # per-file pragma: 0.5.16 anchor + 0.8 tests +evm_version = "istanbul" +optimizer = true +optimizer_runs = 200 +bytecode_hash = "none" +# deployCode reads compiled artifacts from out-forge/ at runtime. +fs_permissions = [{ access = "read", path = "out-forge" }] +remappings = [ + "forge-std/=lib/forge-std/src/", + "@venusprotocol/governance-contracts/=node_modules/@venusprotocol/governance-contracts/", + "@venusprotocol/solidity-utilities/=node_modules/@venusprotocol/solidity-utilities/", + "@openzeppelin/=node_modules/@openzeppelin/", +] + +[profile.default.fuzz] +runs = 256 + +[profile.default.invariant] +runs = 256 +depth = 50 +fail_on_revert = false # handlers guard inputs; reverts are skipped calls + +[profile.ci.fuzz] +runs = 5000 + +[profile.ci.invariant] +runs = 1000 +depth = 100 + +[profile.deep.invariant] +runs = 3000 +depth = 200 + +[rpc_endpoints] +bscmainnet = "${ARCHIVE_NODE_bscmainnet}" diff --git a/test-fuzz/README.md b/test-fuzz/README.md new file mode 100644 index 000000000..e241b7cd4 --- /dev/null +++ b/test-fuzz/README.md @@ -0,0 +1,181 @@ +# XVSVault Fuzz / Invariant Suite (Foundry) + +Adversarial invariant + fuzz testing for the BSC `XVSVault`, focused on the +manipulation surfaces flagged in the security review — above all **minting +voting power** (turning 300k staked XVS into >300k votes), plus solvency, +withdrawal-lock, and pending-withdrawal accounting. + +This is a **Foundry layer bolted onto the Hardhat repo**. It does not touch the +Hardhat setup: sources are scoped to `test-fuzz/`, and build output goes to +`out-forge/` / `cache-forge/` (both gitignored). + +## Layout + +``` +foundry.toml # scoped config; out-forge/cache-forge; remappings +lib/forge-std/ # vendored (gitignored) +test-fuzz/ + anchors/Anchor.sol # 0.5.16 anchor: compiles XVSVaultScenario+XVSStore into artifacts + mocks/MockBEP20.sol # stake+reward token (voting power is amount-based) + mocks/MockACM.sol # always-allow ACM (only isAllowedToCall is used) + interfaces/IXVSVault.sol # 0.8 view of the 0.5.16 vault + XVSVaultTestBase.sol # deploy+wire (BSC pool 0: XVS/XVS, 7d lock, block-based) + aggregates + handlers/VaultHandler.sol # invariant driver (bounded random actions + adversarial ones) + invariants/XVSVaultInvariants.t.sol + invariants/CrossUserIntegrity.t.sol # X1/X2: victim untouched, no principal inflation + invariants/RewardSolvency.t.sol # reward emission cap + no reward-debt underflow + invariants/LegacyPathInvariants.t.sol # legacy (beforeUpgrade) path: solvency + vote conservation + handlers/LegacyVaultHandler.sol # driver mixing legacy + new requests + scenarios/VoteInflation.t.sol + scenarios/DelegateBySig.t.sol # I11/X4/X5: relayed-once, replay, expiry, chainId, malleability, victim-forge + scenarios/RewardIntegrity.t.sol # X3/X6/X9: claim + vault-debt integrity + scenarios/RewardDebt.t.sol # reward-debt integrity across withdrawal lifecycle + scenarios/MultiPool.t.sol # second pool: vote + reward isolation + scenarios/VoteOverflow.t.sol # X7: uint96 vote-cap guards + scenarios/Solc0516Hacks.t.sol # H1-H5: PoC attempts of the canonical pre-0.8 hack classes (all blocked) + scenarios/LegacyPath.t.sol # LG1/LG2/LG_GUARD: legacy beforeUpgrade branch + its guard + fork/ForkLiveHacks.t.sol # H1-H5 replayed against the LIVE bscmainnet vault bytecode + Smoke.t.sol +``` + +Why `deployCode`: forge-std requires solc ≥0.6, so test files are 0.8 while the +vault is 0.5.16. The 0.8 tests instantiate the 0.5.16 contracts via +`deployCode(...)` and interact through `IXVSVault`. The anchor forces the +0.5.16 graph to compile into named artifacts. + +## Running + +Use `--offline` (all solc versions are already installed; no network): + +```bash +cd venus-protocol +forge test --offline -vv # everything +forge test --offline --match-path "test-fuzz/invariants/*" +forge test --offline --match-path "test-fuzz/scenarios/*" +FOUNDRY_PROFILE=deep forge test --offline --match-path "test-fuzz/invariants/*" # 3000×200 +forge test --offline --match-contract ForkLiveHacksTest # fork; needs ARCHIVE_NODE_bscmainnet in .env +``` + +The fork suite (`fork/ForkLiveHacks.t.sol`) runs the H1-H5 hack PoCs against the +**live** bscmainnet vault (proxy `0x0511…9204`, impl `0x74c8…B378`, real XVS + +store) instead of a local copy. It funds attacker wallets via `deal` and skips +automatically when `ARCHIVE_NODE_bscmainnet` is unset. Note: live pool 0 is the +Prime pool, so `deposit`/`requestWithdrawal` call `primeToken.xvsUpdated()`, +which reverts on a fork — the suite `vm.mockCall`s that hook to a no-op to +isolate the vault's own logic (Prime's safety is out of scope here). + +## What it checks + +Invariants (stateful, `XVSVaultInvariants.t.sol`) — hold after every handler call: + +| Id | Property | +| --- | --------------------------------------------------------------------------- | +| I1 | solvency: `balanceOf(vault) >= Σ user.amount` | +| I2 | `totalPendingWithdrawals == Σ user.pendingWithdrawals` | +| I3 | `user.pendingWithdrawals <= user.amount` | +| V1 | vote conservation: `Σ currentVotes == Σ (amount − pending)` over delegators | +| V4 | vote solvency: `Σ currentVotes <= balanceOf(vault) − totalPending` | +| R2 | emission cap: store payout `<= rewardPerBlock * elapsedBlocks` | +| R1b | `pendingReward` never reverts (no reward-debt underflow -> no user DoS) | + +Vote-inflation scenarios (`VoteInflation.t.sol`) — the delta/absolute seam: + +| Id | Attack | +| --- | ------------------------------------------------------------------------------- | +| S1 | re-delegate after a partial withdrawal request (delta vs absolute) | +| S2 | re-delegate to the same delegatee (must net zero) | +| S3 | deposits while undelegated grant 0 votes; one delegate == stake | +| S4 | same-block op storm (checkpoint overwrite must not sum) | +| S5 | historical `getPriorVotes` snapshot ≤ stake at that block (governance-relevant) | +| S6 | `executeWithdrawal` is vote-neutral (no re-burn / no re-add) | + +Reward + config scenarios: + +| Id | File | Property | +| ----- | ------------------ | ----------------------------------------------------------------------------- | +| R1a–c | `RewardDebt.t.sol` | reward-debt stays consistent through request/execute/claim (no underflow DoS) | +| M1 | `MultiPool.t.sol` | rewards isolated per pool (no cross-pool drain) | +| M2 | `MultiPool.t.sol` | only the XVS-staked pool grants votes; a second pool is not a vote backdoor | + +The handler also includes adversarial actions: `donate` (raw transfer bypassing +`deposit`, probing the balance-based reward-supply path) and `warpRoll` +(advances blocks+time in lockstep, mirroring BSC ~3s blocks, and clears the +7-day lock so `executeWithdrawal` is reachable). + +Adversarial-holder suite (threat model: 2-3 colluding accounts that already own +XVS, trying to steal others' funds or manipulate the system through the public +API only — admin/governance surfaces are out of scope): + +Cross-user integrity (stateful, `invariants/CrossUserIntegrity.t.sol`) — a passive +victim stakes+self-delegates, then an attackers-only handler drives the vault: + +| Id | Property | +| --- | ------------------------------------------------------------------------------------------------- | +| X1 | victim's `amount` / `pendingWithdrawals` / `currentVotes` are frozen against all attacker actions | +| X2 | `Σ attacker withdrawn principal <= Σ attacker deposited principal` | + +delegateBySig attacks (`scenarios/DelegateBySig.t.sol`) — a holder cannot move +another account's votes with a forged/stale signature: + +| Id | Attack | +| ---- | ---------------------------------------------------------------------------- | +| X4 | a relayed signature delegates once; replay rejected (nonce consumed) | +| X5a | expired signature rejected (`signature expired`) | +| X5b | wrong-chainId signature cannot move the signer's votes | +| X5c | malleable high-s signature rejected by ECDSA (`invalid signature 's' value`) | +| I11d | a forged payload only ever affects the signer, never a victim | + +Reward-path attacks (`scenarios/RewardIntegrity.t.sol`) — rewards paid from the +separate store; a holder cannot mint, redirect, or double-collect: + +| Id | Attack | +| --- | --------------------------------------------------------------------------------- | +| X3a | a second claim in the same block yields nothing (no double-collect) | +| X3b | `claim(account)` credits that account, not the caller | +| X6 | underfunded-store debt (`pendingRewardTransfers`) repays exactly once, never more | +| X9 | a requested (pending) slice stops earning reward | + +Vote-cap overflow (`scenarios/VoteOverflow.t.sol`) — attackers minted XVS and may +hold balances near the uint96 vote cap: + +| Id | Attack | +| --- | ------------------------------------------------------------------ | +| X7a | a deposit `>= 2^96` reverts on the vote-move overflow guard | +| X7b | accumulating sub-cap deposits to `>= 2^96` then delegating reverts | + +Solidity-0.5.x hack PoCs (`scenarios/Solc0516Hacks.t.sol`, replayed on live +bytecode in `fork/ForkLiveHacks.t.sol`) — each _performs_ a canonical pre-0.8 +attack and asserts it is blocked (a failed exploit is the proof the mitigation +holds). All blocked; no valid hack found. (H3 signature-malleability is covered +by `DelegateBySig.t.sol::X5c` and the fork suite, not duplicated locally.) + +| Id | Pre-0.8 hack class | Attack performed | Guard | +| --- | -------------------------- | ---------------------------------------------------------------- | ----------------------------------------------------------------- | +| H1 | integer underflow | withdraw `stake + 1` (pre-0.8 wraps `amount` to ~2^256) | amount guard reverts; stake intact | +| H2 | reward-debt wrap → mint | churn deposit/request/execute to wrap `.sub` and drain the store | SafeMath reverts; store never over-drained | +| H4 | `ecrecover(0)` forge | fabricated low-s sig to empower a chosen delegatee | recovers an uncontrollable stakeless phantom; target gets 0 votes | +| H5 | Compound/Venus double-vote | withdraw + move XVS to a 2nd wallet to vote the same coins twice | votes burned at request; total == staked, not 2× | + +## Status + +All tests pass. No insolvency, vote inflation, lock bypass, cross-user theft, +reward-debt underflow, reward mint/double-collect, store-debt double-pay, +cross-pool leak, signature replay, or accounting drift found. Consistent with +the manual audits: the current 0.5.16 vault has no exploitable path for an XVS +holder to manipulate XVS or steal another user's funds. + +## Extending + +- **donate reward-share dilution (X8):** assert a raw donation only dilutes the + reward rate and is never claimable as principal. +- **Reward-drain ghost (I8):** add a running `accrued` tally in the handler and + assert `Σ claimed <= accrued` (R2 already bounds store payout by the schedule). +- **Planned withdraw-to-target upgrade:** when that impl exists, add a scenario + asserting I1/I2/V1 still hold post-seizure (a blanket `transfer(target, balanceOf)` + will break I1), and that only the Timelock can call it. +- **Fork mode:** an optional test using `vm.createSelectFork(vm.envString("ARCHIVE_NODE_bscmainnet"))` + against the live impl `0x74c8…B378` for exact-bytecode fidelity. + +``` + +``` diff --git a/test-fuzz/Smoke.t.sol b/test-fuzz/Smoke.t.sol new file mode 100644 index 000000000..751d13a7d --- /dev/null +++ b/test-fuzz/Smoke.t.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "./XVSVaultTestBase.sol"; + +/// @notice Validates deploy/wiring and the core deposit → delegate → request → +/// execute lifecycle before the heavier invariant/scenario suites run. +contract SmokeTest is XVSVaultTestBase { + function setUp() public { + _deployAndWire(); + } + + function test_wiring() public view { + assertEq(vault.poolLength(address(xvs)), 1, "pool 0 not created"); + assertEq(vault.isTimeBased(), false, "should be block-based"); + } + + function test_deposit_delegate_votes() public { + address a = actors[0]; + + vm.prank(a); + vault.deposit(address(xvs), 0, 1000e18); + assertEq(_amountOf(a), 1000e18, "deposit not credited"); + // No votes before delegation (Compound semantics). + assertEq(vault.getCurrentVotes(a), 0, "votes before delegate"); + + vm.prank(a); + vault.delegate(a); + assertEq(uint256(vault.getCurrentVotes(a)), 1000e18, "votes != stake after delegate"); + } + + function test_request_burns_votes_then_execute() public { + address a = actors[0]; + + vm.prank(a); + vault.deposit(address(xvs), 0, 1000e18); + vm.prank(a); + vault.delegate(a); + + vm.prank(a); + vault.requestWithdrawal(address(xvs), 0, 400e18); + // Request immediately removes voting power for the requested amount. + assertEq(uint256(vault.getCurrentVotes(a)), 600e18, "votes not burned on request"); + assertEq(_pendingOf(a), 400e18, "pending not tracked"); + + // Cannot execute before the lock elapses (reverts, nothing eligible). + vm.prank(a); + vm.expectRevert(bytes("nothing to withdraw")); + vault.executeWithdrawal(address(xvs), 0); + assertEq(_amountOf(a), 1000e18, "withdrew before lock"); + + // After the lock, the 400 is withdrawable. + vm.warp(block.timestamp + LOCK_PERIOD + 1); + uint256 balBefore = xvs.balanceOf(a); + vm.prank(a); + vault.executeWithdrawal(address(xvs), 0); + assertEq(_amountOf(a), 600e18, "amount not reduced after execute"); + assertEq(xvs.balanceOf(a) - balBefore, 400e18, "tokens not returned"); + } +} diff --git a/test-fuzz/XVSVaultTestBase.sol b/test-fuzz/XVSVaultTestBase.sol new file mode 100644 index 000000000..a3042d1f0 --- /dev/null +++ b/test-fuzz/XVSVaultTestBase.sol @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { Test } from "forge-std/Test.sol"; +import { IXVSVault, IXVSStore } from "./interfaces/IXVSVault.sol"; +import { MockBEP20 } from "./mocks/MockBEP20.sol"; +import { MockACM } from "./mocks/MockACM.sol"; + +/// @notice Shared deployment + wiring for the XVSVault fuzz/invariant rig. +/// Deploys a locally-instantiated XVSVaultScenario (the 0.5.16 contract, via +/// deployCode) wired exactly as BSC pool 0: XVS as both stake and reward token, +/// 7-day lock, block-based time (mirrors BSC). Exposes a fixed actor set and +/// aggregate accessors reused by the invariants. +abstract contract XVSVaultTestBase is Test { + IXVSVault internal vault; + IXVSStore internal store; + MockBEP20 internal xvs; + MockACM internal acm; + + address[] internal actors; + + uint256 internal constant REWARD_PER_BLOCK = 1e18; + uint256 internal constant LOCK_PERIOD = 7 days; // 604800, matches BSC pool 0 + uint256 internal constant BLOCKS_PER_YEAR = 70080000; // BSC 3s blocks + uint256 internal constant STORE_FUNDING = 1_000_000e18; + uint256 internal constant ACTOR_SEED = 100_000e18; + uint256 internal constant NUM_ACTORS = 5; + + function _deployAndWire() internal { + xvs = new MockBEP20(); + acm = new MockACM(); + + // 0.5.16 contracts via deployCode (see anchors/Anchor.sol). + vault = IXVSVault(deployCode("XVSVaultScenario.sol:XVSVaultScenario")); + store = IXVSStore(deployCode("XVSStore.sol:XVSStore")); + + // Block-based to mirror BSC (isTimeBased == false). + vault.initializeTimeManager(false, BLOCKS_PER_YEAR); + vault.setAccessControl(address(acm)); + + store.setNewOwner(address(vault)); + vault.setXvsStore(address(xvs), address(store)); + store.setRewardToken(address(xvs), true); + + // pool 0: rewardToken = xvs, token = xvs. + vault.add(address(xvs), 100, address(xvs), REWARD_PER_BLOCK, LOCK_PERIOD); + + // Fund the reward store. + xvs.mint(address(store), STORE_FUNDING); + + // Seed actors and pre-approve the vault (deposits pull via transferFrom). + for (uint256 i = 0; i < NUM_ACTORS; i++) { + address a = address(uint160(0x1000 + i)); + actors.push(a); + xvs.mint(a, ACTOR_SEED); + vm.prank(a); + xvs.approve(address(vault), type(uint256).max); + } + + // Advance one block so the pool has a non-zero history before actions. + vm.roll(block.number + 1); + vm.warp(block.timestamp + 3); + } + + // --- aggregate accessors (used by invariants) --- + + function _stakeOf(address a) internal view returns (uint256) { + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, a); + return amount - pending; // getStakeAmount() + } + + function _amountOf(address a) internal view returns (uint256 amount) { + (amount, , ) = vault.getUserInfo(address(xvs), 0, a); + } + + function _pendingOf(address a) internal view returns (uint256 pending) { + (, , pending) = vault.getUserInfo(address(xvs), 0, a); + } + + function _sumAmount() internal view returns (uint256 s) { + for (uint256 i = 0; i < actors.length; i++) s += _amountOf(actors[i]); + } + + function _sumPending() internal view returns (uint256 s) { + for (uint256 i = 0; i < actors.length; i++) s += _pendingOf(actors[i]); + } + + function _sumCurrentVotes() internal view returns (uint256 s) { + for (uint256 i = 0; i < actors.length; i++) s += vault.getCurrentVotes(actors[i]); + } + + /// @notice Σ over delegators of (amount − pending): the stake that *should* + /// back live votes. Votes only accrue once an account has delegated. + function _sumDelegatedStake() internal view returns (uint256 s) { + for (uint256 i = 0; i < actors.length; i++) { + if (vault.delegates(actors[i]) != address(0)) s += _stakeOf(actors[i]); + } + } +} diff --git a/test-fuzz/anchors/Anchor.sol b/test-fuzz/anchors/Anchor.sol new file mode 100644 index 000000000..c52a23521 --- /dev/null +++ b/test-fuzz/anchors/Anchor.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity 0.5.16; +pragma experimental ABIEncoderV2; + +// Compilation anchor: forces Foundry to compile the 0.5.16 XVSVault graph into +// named artifacts so the 0.8 tests can instantiate them via `deployCode`. +// Importing XVSVaultScenario transitively pulls in XVSVault, XVSVaultStorage, +// TimeManagerV5, AccessControlledV5, SafeMath, etc. Nothing here is deployed +// directly; it exists only to seed the artifact set. +import "../../contracts/test/XVSVaultScenario.sol"; +import "../../contracts/XVSVault/XVSStore.sol"; diff --git a/test-fuzz/audits/pocs/AuditFindingsForkPoC.t.sol b/test-fuzz/audits/pocs/AuditFindingsForkPoC.t.sol new file mode 100644 index 000000000..d59d92b49 --- /dev/null +++ b/test-fuzz/audits/pocs/AuditFindingsForkPoC.t.sol @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { Test } from "forge-std/Test.sol"; +import { IXVSVault } from "../../interfaces/IXVSVault.sol"; + +interface IERC20Like { + function approve(address, uint256) external returns (bool); +} + +/// @notice Fork PoC for finding L3 (governance vote-snapshot timing), shown at +/// the deployed-vault primitive that GovernorBravo relies on: getPriorVotes. +/// +/// GovernorBravo snapshots vote weight at proposal.startBlock = creationBlock + +/// votingDelay (GovernorBravoDelegate.sol:509,264), NOT at creation. So voting +/// power delegated AFTER a proposal is created but BEFORE its startBlock is +/// counted. This PoC proves, against the live bscmainnet XVSVault bytecode, that +/// getPriorVotes(actor, snapshot) includes a delegation made after "creation" +/// but before "snapshot", while a snapshot taken at creation would exclude it. +/// +/// This is a Low: it does not bypass the immutable 1.5M quorum — it only widens +/// the window in which weight can be assembled. Gated on ARCHIVE_NODE_bscmainnet. +contract AuditFindingsForkPoC is Test { + IXVSVault internal constant VAULT = IXVSVault(0x051100480289e704d20e9DB4804837068f3f9204); + address internal constant XVS_ADDR = 0xcF6BB5389c92Bdda8a3747Ddb454cB7a64626C63; + + function setUp() public { + string memory rpc = vm.envOr("ARCHIVE_NODE_bscmainnet", string("")); + if (bytes(rpc).length == 0) { + vm.skip(true); + return; + } + vm.createSelectFork(rpc); + if (VAULT.vaultPaused()) { + vm.skip(true); + return; + } + // Isolate the vault primitive from the live Prime hook (see L1). + vm.mockCall(VAULT.primeToken(), abi.encodeWithSignature("xvsUpdated(address)"), bytes("")); + } + + function test_L3_priorVotesCountsDelegationMadeAfterCreationBeforeSnapshot() public { + address actor = makeAddr("late-delegator"); + deal(XVS_ADDR, actor, 50_000e18); + vm.prank(actor); + IERC20Like(XVS_ADDR).approve(address(VAULT), type(uint256).max); + + // t0 = the block a proposal is "created". At this point the actor has no votes. + uint256 creationBlock = block.number; + + // The actor stakes + delegates AFTER creation (simulating assembling weight + // in response to a pending proposal), one block later. + vm.roll(block.number + 1); + vm.warp(block.timestamp + 3); + vm.prank(actor); + VAULT.deposit(XVS_ADDR, 0, 50_000e18); + vm.prank(actor); + VAULT.delegate(actor); + uint256 delegationBlock = block.number; + + // "snapshot" = creation + votingDelay; roll past it so it's queryable. + vm.roll(block.number + 10); + vm.warp(block.timestamp + 30); + uint256 snapshotBlock = delegationBlock + 1; // any block >= delegationBlock + + // FINDING: weight delegated after creation is counted at the snapshot. + uint256 atSnapshot = uint256(VAULT.getPriorVotes(actor, snapshotBlock)); + assertEq(atSnapshot, 50_000e18, "L3: late delegation not counted at snapshot"); + + // CONTRAST: had the snapshot been taken at creation, it would be excluded. + uint256 atCreation = uint256(VAULT.getPriorVotes(actor, creationBlock)); + assertEq(atCreation, 0, "L3: creation-time snapshot would (wrongly) include it"); + } +} diff --git a/test-fuzz/audits/pocs/AuditFindingsPoC.t.sol b/test-fuzz/audits/pocs/AuditFindingsPoC.t.sol new file mode 100644 index 000000000..418f0680e --- /dev/null +++ b/test-fuzz/audits/pocs/AuditFindingsPoC.t.sol @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../../XVSVaultTestBase.sol"; +import { IXVSStore } from "../../interfaces/IXVSVault.sol"; +import { MockBEP20 } from "../../mocks/MockBEP20.sol"; + +/// @notice A Prime token whose xvsUpdated always reverts — stands in for a +/// broken / paused / mis-upgraded Prime (or a Prime market added before its PLP +/// token was initialized), which is the trigger described in finding L1. +contract MockRevertingPrime { + function xvsUpdated(address) external pure { + revert("prime down"); + } +} + +/// @notice Runnable proof-of-concepts for the vault-scope findings in +/// test-fuzz/audits/xvs-vault-findings.md. Each test *demonstrates* the finding +/// and asserts its exact impact + boundary. These are the deployed-vault logic +/// (0.5.16 XVSVaultScenario) exercised locally; the governance-scope findings +/// (L3/L4/L5) are in AuditFindingsForkPoC.t.sol and the report's appendix. +contract AuditFindingsPoC is XVSVaultTestBase { + address internal A; + address internal attacker; + + function setUp() public { + _deployAndWire(); + A = actors[0]; + attacker = actors[1]; + } + + function _warpPastLock() internal { + vm.warp(block.timestamp + LOCK_PERIOD + 1); + vm.roll(block.number + 1); + } + + // ===================================================================== + // L1 — Prime xvsUpdated hook is a hard dependency of deposit & + // requestWithdrawal. A reverting Prime blocks NEW deposits and unstake + // requests, but in-flight withdrawals and claims still complete (no loss). + // ===================================================================== + function test_L1_revertingPrimeFreezesDepositAndRequest_notExecuteOrClaim() public { + // Set up a stake and an in-flight withdrawal request BEFORE Prime is wired. + vm.prank(A); + vault.deposit(address(xvs), 0, 1_000e18); + vm.prank(A); + vault.delegate(A); + vm.prank(A); + vault.requestWithdrawal(address(xvs), 0, 400e18); // in-flight request + + // Wire pool 0 as the Prime pool with a Prime that reverts (the L1 trigger). + MockRevertingPrime badPrime = new MockRevertingPrime(); + vault.setPrimeToken(address(badPrime), address(xvs), 0); + + // IMPACT: new deposit and new withdrawal-request both revert — users are + // frozen out of starting an unstake. + vm.prank(A); + vm.expectRevert(bytes("prime down")); + vault.deposit(address(xvs), 0, 100e18); + + vm.prank(A); + vm.expectRevert(bytes("prime down")); + vault.requestWithdrawal(address(xvs), 0, 100e18); + + // BOUNDARY (no fund loss): executeWithdrawal and claim do NOT call the + // hook, so the already-requested 400 still comes out and rewards claim. + _warpPastLock(); + uint256 balBefore = xvs.balanceOf(A); + vm.prank(A); + vault.executeWithdrawal(address(xvs), 0); + assertEq( + xvs.balanceOf(A) - balBefore, + 400e18, + "L1: in-flight withdrawal blocked (would be worse than reported)" + ); + + vm.prank(A); + vault.claim(A, address(xvs), 0); // must not revert + } + + // ===================================================================== + // L2 — XVSStore.emergencyRewardWithdraw bypasses the reward-token allowlist + // and balance cap. Whoever is store `owner` can sweep ANY token, ANY amount. + // Reachable only if the Timelock repoints the owner (centralization). + // ===================================================================== + function test_L2_emergencyWithdrawDrainsAnyTokenNoAllowlistNoCap() public { + IXVSStore s = IXVSStore(address(store)); + // Sanity: the test contract is the store admin (deployer); owner is the vault. + assertEq(s.admin(), address(this), "L2: precondition"); + + // A non-reward token sitting in the store (e.g. an accidental transfer). + MockBEP20 other = new MockBEP20(); + other.mint(address(store), 5_000e18); + assertEq(s.rewardTokens(address(other)), false, "L2: other is not an allowlisted reward token"); + + // Centralization step: Timelock (here, the admin) repoints owner to attacker. + s.setNewOwner(attacker); + + // DRAIN 1: the non-allowlisted token — emergencyRewardWithdraw has no + // rewardTokens[] check (unlike safeRewardTransfer), so it succeeds. + vm.prank(attacker); + s.emergencyRewardWithdraw(address(other), 5_000e18); + assertEq(other.balanceOf(attacker), 5_000e18, "L2: non-reward token not drained"); + + // DRAIN 2: the entire XVS reward reserve, no balance cap. (attacker was + // pre-seeded XVS by the base rig, so assert the DELTA, not the balance.) + uint256 reserve = xvs.balanceOf(address(store)); + uint256 attackerXvsBefore = xvs.balanceOf(attacker); + vm.prank(attacker); + s.emergencyRewardWithdraw(address(xvs), reserve); + assertEq(xvs.balanceOf(attacker) - attackerXvsBefore, reserve, "L2: reward reserve not drained"); + assertEq(xvs.balanceOf(address(store)), 0, "L2: store not emptied"); + } + + // ===================================================================== + // I1 — Donating the staked token to the vault only DILUTES rewards. The + // donor is not credited any stake and gains nothing. Because both + // pendingReward and _updatePool read the live balanceOf, a donation inflates + // supply = balanceOf - totalPendingWithdrawals and dilutes the (uncommitted) + // accrual — the staker's pending drops the instant the donation lands. + // Griefing at most (attacker pays, undistributed reward stays in the store); + // no theft, no stake credited to the donor. + // ===================================================================== + function test_I1_donationDilutesRewardsNoTheft() public { + vm.prank(A); + vault.deposit(address(xvs), 0, 1_000e18); + + // Accrue a window (uncommitted — no state-changing call runs _updatePool). + vm.roll(block.number + 1_000); + vm.warp(block.timestamp + 3_000); + uint256 pendingBefore = vault.pendingReward(address(xvs), 0, A); + assertGt(pendingBefore, 0, "I1: no baseline accrual"); + + // Attacker donates a large amount of XVS straight to the vault. + uint256 donorBefore = xvs.balanceOf(attacker); + (uint256 donorAmtBefore, , ) = vault.getUserInfo(address(xvs), 0, attacker); + vm.prank(attacker); + xvs.transfer(address(vault), 100_000e18); + + // The donation credits the donor NO stake (pure loss to the donor). + (uint256 donorAmtAfter, , ) = vault.getUserInfo(address(xvs), 0, attacker); + assertEq(donorAmtAfter, donorAmtBefore, "I1: donation wrongly credited donor stake"); + assertEq(xvs.balanceOf(attacker), donorBefore - 100_000e18, "I1: donor not debited"); + + // Same block: A's pending is now DILUTED (supply inflated by the donation). + uint256 pendingAfter = vault.pendingReward(address(xvs), 0, A); + assertLt(pendingAfter, pendingBefore, "I1: donation did not dilute pending reward"); + } + + // ===================================================================== + // I2 — requestWithdrawal calls _transferReward unconditionally (even when + // pending == 0). Harmless: no revert, no double-pay; opportunistically + // settles any prior debt. + // ===================================================================== + function test_I2_requestWithdrawalWithZeroPendingIsSafe() public { + vm.prank(A); + vault.deposit(address(xvs), 0, 1_000e18); + + // Same block as deposit -> no reward accrued yet -> pending == 0. + assertEq(vault.pendingReward(address(xvs), 0, A), 0, "I2: unexpected accrual"); + + uint256 balBefore = xvs.balanceOf(A); + vm.prank(A); + vault.requestWithdrawal(address(xvs), 0, 100e18); // must not revert + // No reward paid (nothing accrued) — no double-pay, no phantom credit. + assertEq(xvs.balanceOf(A), balBefore, "I2: paid a reward with zero pending"); + } +} diff --git a/test-fuzz/audits/xvs-vault-findings.md b/test-fuzz/audits/xvs-vault-findings.md new file mode 100644 index 000000000..6cfdb2dd5 --- /dev/null +++ b/test-fuzz/audits/xvs-vault-findings.md @@ -0,0 +1,213 @@ +# XVS Vault & Governance — Findings Report + +**Date:** 2026-07-09 +**Scope:** deployed bscmainnet XVSVault (proxy `0x051100480289e704d20e9DB4804837068f3f9204`, impl `0x74c8a97BE672db3e9a224648bE566AdA5F43B378`, solc 0.5.16), XVSStore, XVSVaultProxy, GovernorBravoDelegate, Timelock, OmnichainGovernanceExecutor, ACM. +**Method:** 2 manual audits, invariant + legacy-path fuzzing (600k calls/invariant), hack PoCs (local + live-fork bytecode), and 5 agent audits (core logic, integration, governance lifecycle, economics, cross-chain periphery). + +## Summary + +| ID | Severity | Title | Attacker-reachable | +| --- | -------- | ----------------------------------------------------------- | --------------------------------------------- | +| L1 | Low | Prime hook can freeze deposits / withdrawal-requests | No — governance misconfig / bad Prime upgrade | +| L2 | Low | `emergencyRewardWithdraw` bypasses allowlist & cap | No — requires Timelock `setNewOwner` | +| L3 | Low | Vote snapshot taken at `startBlock` (post-delay) | No — still bounded by 1.5M quorum | +| L4 | Low | `castVoteBySig` ballot has no nonce/expiry | No — bounded by `hasVoted` | +| L5 | Low | Cross-chain dedup guard vacuous for proposal id 0 | No — trusted-remote + 3-timelock gated | +| I1 | Info | Donation to vault dilutes rewards (no theft) | n/a | +| I2 | Info | `requestWithdrawal` calls `_transferReward` unconditionally | n/a | + +**No Critical / High / Medium findings.** No unprivileged attacker path to steal, drain, inflate, or freeze-with-loss. + +### Proof-of-concept index + +Every finding has a PoC. Vault-scope PoCs are runnable in this repo; governance-contracts-scope PoCs (L4, L5) are documented in the Appendix (they require the Governor/executor + LayerZero harness that lives in the `governance-contracts` repo). + +| ID | PoC | Where | +| --- | ------------------------------------------------------------------- | -------------------------------------------------------------- | +| L1 | `test_L1_revertingPrimeFreezesDepositAndRequest_notExecuteOrClaim` | `test-fuzz/audits/pocs/AuditFindingsPoC.t.sol` (local) | +| L2 | `test_L2_emergencyWithdrawDrainsAnyTokenNoAllowlistNoCap` | `test-fuzz/audits/pocs/AuditFindingsPoC.t.sol` (local) | +| L3 | `test_L3_priorVotesCountsDelegationMadeAfterCreationBeforeSnapshot` | `test-fuzz/audits/pocs/AuditFindingsForkPoC.t.sol` (live fork) | +| L4 | documented PoC | Appendix A | +| L5 | documented PoC | Appendix B | +| I1 | `test_I1_donationDilutesRewardsNoTheft` | `test-fuzz/audits/pocs/AuditFindingsPoC.t.sol` (local) | +| I2 | `test_I2_requestWithdrawalWithZeroPendingIsSafe` | `test-fuzz/audits/pocs/AuditFindingsPoC.t.sol` (local) | + +Run: `forge test --offline --match-path "test-fuzz/audits/pocs/*"` (the fork PoC skips unless `ARCHIVE_NODE_bscmainnet` is set). + +--- + +## L1 — Prime `xvsUpdated` hook is a hard, revert-propagating dependency of deposit & requestWithdrawal + +- **Severity:** Low (Likelihood Low, Impact High-but-recoverable) +- **Location:** `contracts/XVSVault/XVSVault.sol:314-316` (deposit), `:517-519` (requestWithdrawal). Chain: `PrimeLeaderboard.sol:167` → `PrimeV2.sol:526/1157` → `accrueInterest` → `PrimeLiquidityProvider.accrueTokens` (`:317`). + +**Description.** Pool 0 is the Prime pool, so `deposit` and `requestWithdrawal` call `primeToken.xvsUpdated(msg.sender)` as their final step, with no try/catch. If that call reverts, the whole vault operation reverts — including `requestWithdrawal`, meaning users cannot _begin_ unstaking. Revert triggers: (1) a Prime market whose underlying was never PLP-initialized (`_ensureTokenInitialized` reverts); (2) Prime market count exceeding `PrimeV2.maxLoopsLimit`; (3) Prime/PrimeLeaderboard upgraded to a reverting impl, or `primeToken` repointed to a hostile contract. + +**Impact.** Temporary freeze of pool-0 `deposit` and `requestWithdrawal`. **No fund loss** — `claim()` and `executeWithdrawal()` do not call the hook, so already-requested withdrawals still execute; principal is never lost, only new unstake requests are blocked until governance fixes Prime. + +**PoC.** `test_L1_revertingPrimeFreezesDepositAndRequest_notExecuteOrClaim` (`test-fuzz/audits/pocs/AuditFindingsPoC.t.sol`). Wires pool 0 to a `MockRevertingPrime` (stands in for a broken/paused/mis-upgraded Prime), then asserts `deposit` and `requestWithdrawal` both revert `"prime down"`, while an in-flight `executeWithdrawal` and a `claim` still succeed — pinning both the freeze and the no-loss boundary. + +**Reachability.** Not attacker-triggerable. `addMarket` self-bounds market count to `maxLoopsLimit`; both live Prime markets (vUSDT, vBTCB) are PLP-initialized. Trigger (1) requires a governance ordering mistake (add market before PLP init); (2)/(3) require ACM/Timelock/proxy-admin action. + +**Recommendation.** Call the hook defensively so Prime problems can never brick vault liveness (the hook is a best-effort score update, not vault-critical): + +```solidity +// instead of: primeToken.xvsUpdated(msg.sender); +(bool ok, ) = address(primeToken).call( + abi.encodeWithSelector(IPrimeV5.xvsUpdated.selector, msg.sender) +); +// optionally emit on !ok; do not revert +``` + +Add a fork test that lists a Prime market with an uninitialized PLP token, then calls `requestWithdrawal`, to demonstrate and regress the coupling. As a minimum non-code control, add "PLP-init before addMarket" to the market-listing VIP checklist. + +--- + +## L2 — `XVSStore.emergencyRewardWithdraw` bypasses the reward-token allowlist and balance cap + +- **Severity:** Low (centralization) +- **Location:** `contracts/XVSVault/XVSStore.sol:124-126` + +**Description.** `emergencyRewardWithdraw(token, amount)` is `onlyOwner` and does an unconditional `safeTransfer` with no `rewardTokens[token]` allowlist check and no balance clamp — unlike `safeRewardTransfer` (`:57-68`). It can sweep any token, any amount, from the store. + +**Impact / reachability.** Store owner is the vault (`0x0511…9204`), and the vault exposes no function that calls `emergencyRewardWithdraw`, so it is currently unreachable. Reaching it requires the Timelock to call `setNewOwner` (`:102`, `onlyAdmin`) and repoint the owner to a malicious address — pure centralization, same trust root that could swap the implementation. + +**PoC.** `test_L2_emergencyWithdrawDrainsAnyTokenNoAllowlistNoCap` (`test-fuzz/audits/pocs/AuditFindingsPoC.t.sol`). Repoints the store owner to an attacker (the centralization step), then drains both a **non-allowlisted** token (which `safeRewardTransfer` would reject) and the entire XVS reserve with no cap — demonstrating the missing allowlist and cap checks. + +**Recommendation.** No action required given current wiring. If tightening: restrict to a dedicated recovery role separate from `owner`, or route through the vault. Documented as the highest-value privileged sink touching the reward store. + +--- + +## L3 — Governance vote weight snapshotted at `startBlock` (after `votingDelay`) + +- **Severity:** Low (defense-in-depth) +- **Location:** `GovernorBravoDelegate.sol:509` (`getPriorVotes(voter, proposal.startBlock)`), `:264` (`startBlock = block.number + votingDelay`) + +**Description.** Vote weight is read at `startBlock` = creation + `votingDelay`, a known future block, not at creation. Standard Compound Bravo behavior; a party can acquire/delegate voting power to be reflected at `startBlock` and vote. + +**Impact / reachability.** Cannot pass a proposal — still needs `forVotes >= 1,500,000` (immutable) and `> againstVotes`. A 327k actor gains nothing beyond honest weight; "borrowing" to swing would need >1.17M delegated XVS (economic, not code) and is visible during the voting period where opposition can vote against. No code gate bypassed. + +**PoC.** `test_L3_priorVotesCountsDelegationMadeAfterCreationBeforeSnapshot` (`test-fuzz/audits/pocs/AuditFindingsForkPoC.t.sol`, runs against **live** bscmainnet bytecode). Demonstrates at the exact primitive the Governor reads: a delegation made _after_ a proposal's creation block but _before_ its `startBlock` is counted by `getPriorVotes(actor, snapshot)`, whereas a creation-block snapshot would return 0. Confirms the front-run window; the 1.5M quorum still bounds impact. + +**Recommendation.** None required. For tighter front-run resistance, snapshot at `block.number - 1` (as the proposer-threshold check already does at `:238`). + +--- + +## L4 — `castVoteBySig` ballot has no nonce or expiry + +- **Severity:** Low (defense-in-depth) +- **Location:** `GovernorBravoDelegate.sol:485-494` + +**Description.** The EIP-712 ballot is `Ballot(uint256 proposalId, uint8 support)` — no nonce, no expiry. A signature is valid forever for that `proposalId`. + +**Impact / reachability.** Harmless in practice: `receipt.hasVoted` (`:508`) blocks double-counting, `proposalId` binds the ballot, and IDs are never reused (monotonic). A relayer can only submit the exact `(proposalId, support)` the signer already chose — cannot change support, double-count, or replay to another proposal. Worst case is timing of when an already-decided vote lands. Signature malleability is a non-issue for the same `hasVoted` reason. + +**PoC.** Documented in **Appendix A** (Foundry test for the `governance-contracts` repo — needs a deployed GovernorBravo + XVSVault vote source, so it does not run in this repo's vault rig). + +**Recommendation.** Defense-in-depth: add `expiry` to the ballot and enforce canonical low-s / `v∈{27,28}`. Note `XVSVault.delegateBySig` already uses nonce+expiry; the governor ballot omits them (matching upstream Bravo). + +--- + +## L5 — OmnichainGovernanceExecutor dedup guard is vacuous for proposal id 0 + +- **Severity:** Low (defense-in-depth / code robustness) +- **Location:** `governance-contracts/contracts/Cross-chain/OmnichainGovernanceExecutor.sol` — `_nonblockingLzReceive`, `:364` (`require(proposals[pId].id == 0)`), with `id: pId` (`:378`) and `proposals[pId] = newProposal` (`:389`). + +**Description.** The replay guard infers "never received" from `proposals[pId].id == 0`. Because a stored proposal sets `id = pId`, for `pId == 0` the guard stays true forever — a second `pId == 0` message would overwrite the prior proposal, resetting `executed`/`canceled` and re-queueing it. + +**Impact / reachability.** Not attacker-triggerable. The message must arrive over an authenticated LayerZero channel from the trusted `OmnichainProposalSender` (execute rights held only by the 3 timelocks), and Compound-style governance proposal IDs are 1-indexed, so `pId == 0` is never naturally produced. Exploitation would require a source already holding governance execute rights — i.e., the attacker already owns governance. + +**PoC.** Documented in **Appendix B** (Foundry test for the `governance-contracts` repo — needs the executor + a mock LayerZero endpoint, so it does not run in this repo's vault rig). + +**Recommendation.** Track receipt with a dedicated `bool received` flag or `mapping(uint256 => bool)` rather than inferring from `.id`, or reject `pId == 0` explicitly. + +--- + +## I1 — Donation of XVS to the vault dilutes rewards (no theft) + +- **Location:** `XVSVault.sol:628-639` (`_updatePool`), `:589-597` (`pendingReward`) + +Pool 0 stakes and rewards XVS, so anyone can `transfer` XVS directly to the vault, inflating `supply = balanceOf - totalPendingWithdrawals`. Because both `pendingReward` and `_updatePool` read the live `balanceOf`, a donation dilutes the **uncommitted** accrual window — a staker's pending reward drops the instant a donation lands (until the next `_updatePool` commit, which also reads the inflated balance). Rewards come from the separate store, so donated tokens are never distributed and undistributed emissions stay in the store. The donor is credited no stake and gets nothing back; at most this is a griefing nudge (attacker pays to shave others' pending), never theft. + +**PoC.** `test_I1_donationDilutesRewardsNoTheft` (`test-fuzz/audits/pocs/AuditFindingsPoC.t.sol`). Accrues a window, donates 100k XVS, and asserts (a) the donor's `user.amount` is unchanged and the donor is debited, and (b) the staker's `pendingReward` strictly decreases after the donation. + +## I2 — `requestWithdrawal` calls `_transferReward` unconditionally + +- **Location:** `XVSVault.sol:499-500` vs. guarded pattern in `deposit` (`:300`) and `claim` (`:337`) + +`requestWithdrawal` always calls `_transferReward(..., pending)` even when `pending == 0`. Beneficial (opportunistically settles outstanding `pendingRewardTransfers` debt), idempotent, no double-pay. Code-quality inconsistency only. + +**PoC.** `test_I2_requestWithdrawalWithZeroPendingIsSafe` (`test-fuzz/audits/pocs/AuditFindingsPoC.t.sol`). Requests a withdrawal in the same block as the deposit (zero pending) and asserts it neither reverts nor pays any phantom reward. + +--- + +## Load-bearing invariants — do not regress + +- `pendingWithdrawalsBeforeUpgrade == 0` guard on deposit/claim/requestWithdrawal (`:296`, `:332`, `:496`) — prevents a mixed legacy/new request state that would permanently freeze a position. +- `setXvsStore` one-time init (`:877`) — the reward store cannot be repointed, even by admin. +- Immutable `quorumVotes = 1500000e18` constant (`GovernorBravoDelegate.sol:84`) — single value shared across all proposal routes; no reduced-quorum path. + +## Follow-up verification (out-of-bundle) + +- Confirm `AccessControlledV8._authorizeUpgrade` enforces owner/ACM for `OmnichainExecutorOwner` UUPS upgrades. (XVSVault storage layout already verified clean — append-only `__gap[46]`, deprecated slots preserved.) + +--- + +## Appendix A — L4 PoC (`castVoteBySig` replayability) + +Belongs in the `governance-contracts` repo (needs a deployed GovernorBravo + an XVSVault vote source). It demonstrates that a signed ballot has no nonce/expiry: the same `(v,r,s)` a signer produced can be relayed by anyone, at any time within the voting window, and — because there is no expiry — it never goes stale. The `hasVoted` guard is what bounds the impact (no double-count, no support change), so the assertion set proves both the missing protections and the containment. + +```solidity +// GovernorBravo already deployed + configured; `signer` holds voting power at startBlock. +function test_L4_ballotHasNoNonceOrExpiry() public { + uint256 pid = _createActiveProposal(); // reach Active state + bytes32 domain = keccak256( + abi.encode(governor.DOMAIN_TYPEHASH(), keccak256(bytes("Venus Governor Bravo")), block.chainid, address(governor)) + ); + // NOTE: Ballot has ONLY (proposalId, support) — no nonce, no expiry field exists. + bytes32 structHash = keccak256(abi.encode(governor.BALLOT_TYPEHASH(), pid, uint8(1))); + bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domain, structHash)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, digest); + + // Anyone can relay it; there is no expiry to enforce and no nonce to consume. + vm.warp(block.timestamp + 365 days); // arbitrarily late — still valid + governor.castVoteBySig(pid, 1, v, r, s); + assertTrue(_hasVoted(pid, signer), "L4: signed ballot accepted with no nonce/expiry"); + + // Containment: a replay of the SAME signature is rejected by hasVoted (not by a nonce). + vm.expectRevert(bytes("GovernorBravo::castVoteInternal: voter already voted")); + governor.castVoteBySig(pid, 1, v, r, s); +} +``` + +**Fix regressed by:** adding `expiry` to the `Ballot` struct + typehash and asserting a past-expiry signature reverts. + +## Appendix B — L5 PoC (`pId == 0` dedup bypass on OmnichainGovernanceExecutor) + +Belongs in the `governance-contracts` repo (needs the executor + a mock LayerZero endpoint). It shows that two trusted-remote messages carrying `pId == 0` both pass the `require(proposals[pId].id == 0)` guard — the second overwrites the first, resetting `executed`/`canceled` and re-queueing. Not reachable in production (proposal IDs are 1-indexed and the channel is trusted-remote from the 3 timelocks), so this is a robustness/hardening PoC. + +```solidity +// Executor deployed with a MockLZEndpoint set as the trusted remote/source. +function test_L5_pIdZeroDedupIsVacuous() public { + bytes memory payload0 = _encodeProposal(/*pId=*/ 0, targets, values, sigs, calldatas, /*route=*/ 0); + + // First delivery of pId==0: stored, guard was proposals[0].id == 0 (true). + _deliverLzMessage(payload0); + (, , bool executed0, bool canceled0) = executor.proposals(0); + + // Cancel it (guardian) so state is terminal. + vm.prank(guardian); + executor.cancel(0); + (, , , bool canceledAfter) = executor.proposals(0); + assertTrue(canceledAfter, "L5: setup — proposal 0 canceled"); + + // FINDING: because proposals[0].id was set to 0 (== pId), the guard is STILL + // true, so a second pId==0 message is accepted and OVERWRITES the canceled one, + // resetting canceled=false and re-queueing. + _deliverLzMessage(payload0); + (, , , bool canceledReplayed) = executor.proposals(0); + assertFalse(canceledReplayed, "L5: pId==0 message overwrote a terminal proposal"); +} +``` + +**Fix regressed by:** switching the guard to a dedicated `mapping(uint256 => bool) received` (or rejecting `pId == 0`), then asserting the second delivery reverts. diff --git a/test-fuzz/fork/ForkLiveHacks.t.sol b/test-fuzz/fork/ForkLiveHacks.t.sol new file mode 100644 index 000000000..2fb992d9e --- /dev/null +++ b/test-fuzz/fork/ForkLiveHacks.t.sol @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { Test } from "forge-std/Test.sol"; +import { IXVSVault } from "../interfaces/IXVSVault.sol"; + +interface IERC20Like { + function balanceOf(address) external view returns (uint256); + function approve(address, uint256) external returns (bool); + function transfer(address, uint256) external returns (bool); +} + +/// @notice The same canonical pre-0.8 hack PoCs as `scenarios/Solc0516Hacks.t.sol`, +/// but replayed against the LIVE bscmainnet XVSVault — real proxy, real 0.5.16 +/// implementation bytecode, real XVS token and reward store — instead of a +/// locally-deployed copy with mocks. This is the realistic check demanded by the +/// active XVS-accumulation threat: prove the deployed contract an attacker would +/// actually hit is not exploitable, not just the repo source. +/// +/// Live targets (deployments/bscmainnet_addresses.json): +/// proxy 0x051100480289e704d20e9DB4804837068f3f9204 +/// impl 0x74c8a97BE672db3e9a224648bE566AdA5F43B378 (solc 0.5.16, Etherscan-verified) +/// XVS 0xcF6BB5389c92Bdda8a3747Ddb454cB7a64626C63 (pool 0 stake + reward) +/// store 0x1e25CF968f12850003Db17E0Dba32108509C4359 +/// +/// Gated on the `ARCHIVE_NODE_bscmainnet` RPC (forge auto-loads it from .env). +/// When it is unset the whole suite skips, mirroring the Hardhat fork convention. +contract ForkLiveHacksTest is Test { + IXVSVault internal constant VAULT = IXVSVault(0x051100480289e704d20e9DB4804837068f3f9204); + IERC20Like internal constant XVS = IERC20Like(0xcF6BB5389c92Bdda8a3747Ddb454cB7a64626C63); + address internal constant STORE = 0x1e25CF968f12850003Db17E0Dba32108509C4359; + address internal constant XVS_ADDR = 0xcF6BB5389c92Bdda8a3747Ddb454cB7a64626C63; + + // secp256k1 curve order n (for the malleable-s counterpart). + uint256 internal constant SECP256K1_N = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141; + bytes32 internal constant DOMAIN_TYPEHASH = + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); + bytes32 internal constant DELEGATION_TYPEHASH = + keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)"); + + uint256 internal constant PID = 0; + uint256 internal constant STAKE = 1_000e18; + + address internal attacker; + address internal w2; + + function setUp() public { + string memory rpc = vm.envOr("ARCHIVE_NODE_bscmainnet", string("")); + if (bytes(rpc).length == 0) { + vm.skip(true); + return; + } + vm.createSelectFork(rpc); + + // A prepared pause could make every user action revert; skip rather than + // report a misleading pass. + if (VAULT.vaultPaused()) { + vm.skip(true); + return; + } + + // Live pool 0 is the Prime pool, so deposit/requestWithdrawal invoke + // primeToken.xvsUpdated(); on a fork that Prime contract reverts + // (NotActivated). Neutralize the hook to a no-op so these tests exercise + // the VAULT's own arithmetic/vote/signature logic in isolation — the + // surface the hacks target. (Prime's own safety is covered separately.) + vm.mockCall(VAULT.primeToken(), abi.encodeWithSignature("xvsUpdated(address)"), bytes("")); + + attacker = makeAddr("attacker"); + w2 = makeAddr("w2"); + _fund(attacker, 100_000e18); + } + + function _fund(address who, uint256 amt) internal { + deal(XVS_ADDR, who, amt); + vm.prank(who); + XVS.approve(address(VAULT), type(uint256).max); + } + + function _digest(address delegatee, uint256 nonce, uint256 expiry) internal view returns (bytes32) { + bytes32 ds = keccak256( + abi.encode(DOMAIN_TYPEHASH, keccak256(bytes("XVSVault")), block.chainid, address(VAULT)) + ); + bytes32 sh = keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry)); + return keccak256(abi.encodePacked("\x19\x01", ds, sh)); + } + + // Advance both blocks (reward accrual is block-based on BSC) and time (the + // withdrawal lock is timestamp-based) well past the pool's 7-day lock. + function _advancePastLock() internal { + vm.roll(block.number + 1_000_000); // ~35 days of 3s blocks + vm.warp(block.timestamp + 30 days); + } + + // ---- H1 (live): withdraw more than staked must revert, not underflow ---- + function test_fork_H1_withdrawOverStakeReverts() public { + vm.prank(attacker); + VAULT.deposit(XVS_ADDR, PID, STAKE); + + vm.prank(attacker); + vm.expectRevert(bytes("requested amount is invalid")); + VAULT.requestWithdrawal(XVS_ADDR, PID, STAKE + 1); + + (uint256 amount, , ) = VAULT.getUserInfo(XVS_ADDR, PID, attacker); + assertEq(amount, STAKE, "H1: live stake corrupted"); + } + + // ---- H2 (live): reward accounting cannot underflow into an infinite mint ---- + function test_fork_H2_noRewardUnderflowMint() public { + uint256 storeBefore = XVS.balanceOf(STORE); + + vm.prank(attacker); + VAULT.deposit(XVS_ADDR, PID, 500e18); + vm.roll(block.number + 100_000); + vm.warp(block.timestamp + 3 days); + + vm.prank(attacker); + VAULT.requestWithdrawal(XVS_ADDR, PID, 200e18); + _advancePastLock(); + vm.prank(attacker); + VAULT.executeWithdrawal(XVS_ADDR, PID); + + uint256 pending = VAULT.pendingReward(XVS_ADDR, PID, attacker); + assertLt(pending, storeBefore, "H2: pending reward exceeds entire store (wrap?)"); + + vm.prank(attacker); + VAULT.claim(attacker, XVS_ADDR, PID); + assertLe(storeBefore - XVS.balanceOf(STORE), storeBefore, "H2: store over-drained"); + } + + // ---- H3 (live): signature-malleability replay on delegateBySig ---- + function test_fork_H3_malleabilityReplayBlocked() public { + uint256 pk = 0xA11CE; + address signer = vm.addr(pk); + _fund(signer, 10_000e18); + vm.prank(signer); + VAULT.deposit(XVS_ADDR, PID, 10_000e18); + + address B = makeAddr("delegatee"); + uint256 expiry = block.timestamp + 1 hours; + uint256 nonce = VAULT.nonces(signer); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, _digest(B, nonce, expiry)); + + // Original relayed delegation moves the signer's staked votes to B. + VAULT.delegateBySig(B, nonce, expiry, v, r, s); + assertEq(uint256(VAULT.getCurrentVotes(B)), 10_000e18, "H3: setup"); + + // Malleable twin of the SAME signature — rejected by the s-value check. + bytes32 sMal = bytes32(SECP256K1_N - uint256(s)); + uint8 vMal = v == 27 ? 28 : 27; + vm.expectRevert(bytes("ECDSA: invalid signature 's' value")); + VAULT.delegateBySig(B, nonce, expiry, vMal, r, sMal); + } + + // ---- H4 (live): forged signature grants the chosen target no votes ---- + function test_fork_H4_ecrecoverForgeGrantsNoVotes() public { + address target = makeAddr("target"); + uint256 expiry = block.timestamp + 1 hours; + try VAULT.delegateBySig(target, 0, expiry, 27, bytes32(uint256(1)), bytes32(uint256(1))) {} catch {} + assertEq(uint256(VAULT.getCurrentVotes(target)), 0, "H4: forged sig moved votes to target"); + } + + // ---- H5 (live): historical double-vote via moving the underlying XVS ---- + function test_fork_H5_doubleVoteViaTransferBlocked() public { + uint256 x = 50_000e18; + _fund(w2, 0); // just set approval; w2 gets XVS from attacker below + + vm.prank(attacker); + VAULT.deposit(XVS_ADDR, PID, x); + vm.prank(attacker); + VAULT.delegate(attacker); + assertEq(uint256(VAULT.getCurrentVotes(attacker)), x, "H5: setup"); + + vm.prank(attacker); + VAULT.requestWithdrawal(XVS_ADDR, PID, x); + assertEq(uint256(VAULT.getCurrentVotes(attacker)), 0, "H5: votes survived request (double-count!)"); + + _advancePastLock(); + vm.prank(attacker); + VAULT.executeWithdrawal(XVS_ADDR, PID); + + vm.prank(attacker); + XVS.transfer(w2, x); + vm.prank(w2); + VAULT.deposit(XVS_ADDR, PID, x); + vm.prank(w2); + VAULT.delegate(w2); + + assertEq(uint256(VAULT.getCurrentVotes(w2)), x, "H5: w2 votes wrong"); + assertEq( + uint256(VAULT.getCurrentVotes(attacker)) + uint256(VAULT.getCurrentVotes(w2)), + x, + "H5: total votes exceed staked XVS (double-vote succeeded)" + ); + } +} diff --git a/test-fuzz/handlers/LegacyVaultHandler.sol b/test-fuzz/handlers/LegacyVaultHandler.sol new file mode 100644 index 000000000..6f9ef90aa --- /dev/null +++ b/test-fuzz/handlers/LegacyVaultHandler.sol @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { CommonBase } from "forge-std/Base.sol"; +import { StdUtils } from "forge-std/StdUtils.sol"; +import { IXVSVault } from "../interfaces/IXVSVault.sol"; +import { MockBEP20 } from "../mocks/MockBEP20.sol"; + +/// @notice Invariant driver that mixes LEGACY (pre-upgrade, afterUpgrade=0) +/// withdrawal requests with the normal action set. The legacy path is +/// structurally different and untested elsewhere: +/// - `requestOldWithdrawal` burns votes but does NOT touch +/// `totalPendingWithdrawals` (so the I2 accounting invariant deliberately +/// does not apply to this handler), +/// - the `executeWithdrawal` beforeUpgrade branch pays reward on the FULL +/// `user.amount` and does NOT call `_moveDelegates`. +/// The goal is to find any sequence that breaks solvency or vote conservation +/// by interleaving legacy + new requests, delegation, and partial executes. +contract LegacyVaultHandler is CommonBase, StdUtils { + IXVSVault internal immutable vault; + MockBEP20 internal immutable xvs; + address[] internal actors; + + uint256 public gDeposited; + uint256 public gWithdrawn; + uint256 public callDeposit; + uint256 public callRequestNew; + uint256 public callRequestOld; + uint256 public callExecute; + uint256 public callDelegate; + + constructor(IXVSVault _vault, MockBEP20 _xvs, address[] memory _actors) { + vault = _vault; + xvs = _xvs; + actors = _actors; + } + + function _actor(uint256 seed) internal view returns (address) { + return actors[bound(seed, 0, actors.length - 1)]; + } + + function _available(address a) internal view returns (uint256) { + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, a); + return amount - pending; + } + + function deposit(uint256 actorSeed, uint256 amt) external { + address a = _actor(actorSeed); + uint256 bal = xvs.balanceOf(a); + if (bal == 0) return; + amt = bound(amt, 1, bal); + vm.prank(a); + // Reverts if the actor has a pending beforeUpgrade request; that's a + // valid guard, so skip rather than force it. + try vault.deposit(address(xvs), 0, amt) { + gDeposited += amt; + callDeposit++; + } catch {} + } + + function delegate(uint256 actorSeed, uint256 dSeed) external { + address a = _actor(actorSeed); + uint256 pick = bound(dSeed, 0, actors.length); + address target = pick == actors.length ? address(0) : actors[pick]; + vm.prank(a); + vault.delegate(target); + callDelegate++; + } + + function requestNew(uint256 actorSeed, uint256 amt) external { + address a = _actor(actorSeed); + uint256 avail = _available(a); + if (avail == 0) return; + amt = bound(amt, 1, avail); + vm.prank(a); + try vault.requestWithdrawal(address(xvs), 0, amt) { + callRequestNew++; + } catch {} + } + + function requestOld(uint256 actorSeed, uint256 amt) external { + address a = _actor(actorSeed); + uint256 avail = _available(a); + if (avail == 0) return; + amt = bound(amt, 1, avail); + vm.prank(a); + try vault.requestOldWithdrawal(address(xvs), 0, amt) { + callRequestOld++; + } catch {} + } + + function executeWithdrawal(uint256 actorSeed) external { + address a = _actor(actorSeed); + uint256 before = xvs.balanceOf(a); + vm.prank(a); + try vault.executeWithdrawal(address(xvs), 0) { + gWithdrawn += xvs.balanceOf(a) - before; + callExecute++; + } catch {} + } + + function claim(uint256 actorSeed) external { + address a = _actor(actorSeed); + vm.prank(a); + try vault.claim(a, address(xvs), 0) {} catch {} + } + + function warpRoll(uint256 secondsSeed) external { + uint256 dt = bound(secondsSeed, 1, 10 days); + vm.warp(block.timestamp + dt); + vm.roll(block.number + (dt / 3) + 1); + } +} diff --git a/test-fuzz/handlers/VaultHandler.sol b/test-fuzz/handlers/VaultHandler.sol new file mode 100644 index 000000000..2a792ab23 --- /dev/null +++ b/test-fuzz/handlers/VaultHandler.sol @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { CommonBase } from "forge-std/Base.sol"; +import { StdUtils } from "forge-std/StdUtils.sol"; +import { IXVSVault } from "../interfaces/IXVSVault.sol"; +import { MockBEP20 } from "../mocks/MockBEP20.sol"; + +/// @notice Invariant fuzzing driver. The fuzzer calls these bounded actions in +/// random sequences against a fixed actor set. Inputs are bounded so calls are +/// meaningful; genuinely-invalid calls revert and are skipped +/// (invariant.fail_on_revert = false). Includes adversarial actions (raw +/// donation, same-block bursts, lock-boundary warps). +contract VaultHandler is CommonBase, StdUtils { + IXVSVault internal immutable vault; + MockBEP20 internal immutable xvs; + address[] internal actors; + + // ghosts + uint256 public gDeposited; + uint256 public gWithdrawn; + uint256 public gClaimed; + uint256 public callDeposit; + uint256 public callRequest; + uint256 public callExecute; + uint256 public callDelegate; + + constructor(IXVSVault _vault, MockBEP20 _xvs, address[] memory _actors) { + vault = _vault; + xvs = _xvs; + actors = _actors; + } + + function _actor(uint256 seed) internal view returns (address) { + return actors[bound(seed, 0, actors.length - 1)]; + } + + function deposit(uint256 actorSeed, uint256 amt) external { + address a = _actor(actorSeed); + uint256 bal = xvs.balanceOf(a); + if (bal == 0) return; + amt = bound(amt, 1, bal); + vm.prank(a); + vault.deposit(address(xvs), 0, amt); + gDeposited += amt; + callDeposit++; + } + + function requestWithdrawal(uint256 actorSeed, uint256 amt) external { + address a = _actor(actorSeed); + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, a); + uint256 avail = amount - pending; + if (avail == 0) return; + amt = bound(amt, 1, avail); + vm.prank(a); + vault.requestWithdrawal(address(xvs), 0, amt); + callRequest++; + } + + function executeWithdrawal(uint256 actorSeed) external { + address a = _actor(actorSeed); + uint256 before = xvs.balanceOf(a); + vm.prank(a); + // Reverts ("nothing to withdraw") when nothing is eligible; skipped. + try vault.executeWithdrawal(address(xvs), 0) { + gWithdrawn += xvs.balanceOf(a) - before; + callExecute++; + } catch {} + } + + function claim(uint256 actorSeed) external { + address a = _actor(actorSeed); + uint256 before = xvs.balanceOf(a); + vm.prank(a); + try vault.claim(a, address(xvs), 0) { + gClaimed += xvs.balanceOf(a) - before; + } catch {} + } + + function delegate(uint256 actorSeed, uint256 delegateeSeed) external { + address a = _actor(actorSeed); + // Include address(0) (undelegate) in the target space. + address target; + uint256 pick = bound(delegateeSeed, 0, actors.length); + target = pick == actors.length ? address(0) : actors[pick]; + vm.prank(a); + vault.delegate(target); + callDelegate++; + } + + /// @notice Advance time and blocks in lockstep (BSC ~3s/block). Occasionally + /// jumps far enough to clear the 7-day lock so executeWithdrawal is reachable. + function warpRoll(uint256 secondsSeed) external { + uint256 dt = bound(secondsSeed, 1, 10 days); + vm.warp(block.timestamp + dt); + vm.roll(block.number + (dt / 3) + 1); + } + + /// @notice Adversarial: raw transfer into the vault (bypassing deposit). + /// Probes the balance-based reward-supply path; must only dilute, never + /// break solvency or let anyone claim the donation as principal. + function donate(uint256 amt) external { + address a = _actor(amt); + uint256 bal = xvs.balanceOf(a); + if (bal == 0) return; + amt = bound(amt, 1, bal); + vm.prank(a); + xvs.transfer(address(vault), amt); + } + + function actorsLength() external view returns (uint256) { + return actors.length; + } +} diff --git a/test-fuzz/interfaces/IXVSVault.sol b/test-fuzz/interfaces/IXVSVault.sol new file mode 100644 index 000000000..ffb07efcd --- /dev/null +++ b/test-fuzz/interfaces/IXVSVault.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +/// @notice 0.8-side view of the 0.5.16 XVSVault (XVSVaultScenario). Only the +/// selectors used by the rig are declared. The concrete contract is deployed +/// via `deployCode` and cast to this interface. +interface IXVSVault { + // --- admin / wiring --- + function initializeTimeManager(bool timeBased_, uint256 blocksPerYear_) external; + function setAccessControl(address newAccessControlAddress) external; + function setXvsStore(address _xvs, address _xvsStore) external; + function add( + address _rewardToken, + uint256 _allocPoint, + address _token, + uint256 _rewardPerBlockOrSecond, + uint256 _lockPeriod + ) external; + function setWithdrawalLockingPeriod(address _rewardToken, uint256 _pid, uint256 _newPeriod) external; + function setPrimeToken(address _primeToken, address _primeRewardToken, uint256 _primePoolId) external; + function pause() external; + function resume() external; + + // --- user actions --- + function deposit(address _rewardToken, uint256 _pid, uint256 _amount) external; + function requestWithdrawal(address _rewardToken, uint256 _pid, uint256 _amount) external; + // XVSVaultScenario-only: fabricates a pre-upgrade (afterUpgrade=0) request to + // exercise the legacy executeWithdrawal branch that current tests never hit. + function requestOldWithdrawal(address _rewardToken, uint256 _pid, uint256 _amount) external; + function executeWithdrawal(address _rewardToken, uint256 _pid) external; + function claim(address _account, address _rewardToken, uint256 _pid) external; + function delegate(address delegatee) external; + function delegateBySig(address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s) external; + + // --- views --- + function getUserInfo( + address _rewardToken, + uint256 _pid, + address _user + ) external view returns (uint256 amount, uint256 rewardDebt, uint256 pendingWithdrawals); + function getCurrentVotes(address account) external view returns (uint96); + function getPriorVotes(address account, uint256 blockNumberOrSecond) external view returns (uint96); + function pendingReward(address _rewardToken, uint256 _pid, address _user) external view returns (uint256); + function totalPendingWithdrawals(address _rewardToken, uint256 _pid) external view returns (uint256); + function pendingRewardTransfers(address _rewardToken, address _user) external view returns (uint256); + function delegates(address account) external view returns (address); + function nonces(address account) external view returns (uint256); + function poolLength(address rewardToken) external view returns (uint256); + function isTimeBased() external view returns (bool); + function vaultPaused() external view returns (bool); + function primeToken() external view returns (address); +} + +interface IXVSStore { + function setNewOwner(address _owner) external; + function setRewardToken(address _tokenAddress, bool status) external; + function emergencyRewardWithdraw(address _tokenAddress, uint256 _amount) external; + function owner() external view returns (address); + function admin() external view returns (address); + function rewardTokens(address) external view returns (bool); +} diff --git a/test-fuzz/invariants/CrossUserIntegrity.t.sol b/test-fuzz/invariants/CrossUserIntegrity.t.sol new file mode 100644 index 000000000..2e6b57760 --- /dev/null +++ b/test-fuzz/invariants/CrossUserIntegrity.t.sol @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; +import { VaultHandler } from "../handlers/VaultHandler.sol"; + +/// @notice Threat model: 2-3 colluding XVS holders (already own XVS) driving the +/// vault through every public action, trying to (a) touch a passive victim's +/// funds/votes, or (b) extract more principal than they put in. A designated +/// victim stakes and self-delegates BEFORE the attackers act; the handler is +/// wired with ONLY the attacker addresses, so the fuzzer can never prank the +/// victim. If any attacker sequence moves the victim's numbers or lets an +/// attacker withdraw more than deposited, the counterexample is a real theft. +/// +/// X1 victim principal/pending/votes are frozen against all attacker actions +/// X2 Σ attacker withdrawn principal <= Σ attacker deposited principal +contract CrossUserIntegrity is XVSVaultTestBase { + VaultHandler internal handler; + + address internal victim; + uint256 internal victimAmount; + uint96 internal victimVotes; + + function setUp() public { + _deployAndWire(); + + // Victim (actors[3]) stakes and self-delegates before any attacker moves. + victim = actors[3]; + vm.prank(victim); + vault.deposit(address(xvs), 0, 50_000e18); + vm.prank(victim); + vault.delegate(victim); + victimAmount = _amountOf(victim); + victimVotes = vault.getCurrentVotes(victim); + + // Attackers-only handler: actors[0..2]. The victim is never a target. + address[] memory attackers = new address[](3); + attackers[0] = actors[0]; + attackers[1] = actors[1]; + attackers[2] = actors[2]; + handler = new VaultHandler(vault, xvs, attackers); + targetContract(address(handler)); + } + + /// X1: no attacker action can change the victim's principal, pending, or votes. + function invariant_X1_victimUntouched() public view { + assertEq(_amountOf(victim), victimAmount, "X1: victim amount changed"); + assertEq(_pendingOf(victim), 0, "X1: victim pending changed"); + assertEq(uint256(vault.getCurrentVotes(victim)), uint256(victimVotes), "X1: victim votes changed"); + } + + /// X2: attackers can never withdraw more principal than they deposited. + /// Reward is paid from the separate store, so principal-out must be bounded + /// by principal-in. + function invariant_X2_noPrincipalInflation() public view { + assertLe(handler.gWithdrawn(), handler.gDeposited(), "X2: withdrew more than deposited"); + } + + /// Coverage guard: surface a run where attackers never actually staked. + function invariant_callSummary() public view { + assertTrue( + handler.callDeposit() + handler.callRequest() + handler.callExecute() + handler.callDelegate() >= 0, + "unreachable" + ); + } +} diff --git a/test-fuzz/invariants/LegacyPathInvariants.t.sol b/test-fuzz/invariants/LegacyPathInvariants.t.sol new file mode 100644 index 000000000..a007d09eb --- /dev/null +++ b/test-fuzz/invariants/LegacyPathInvariants.t.sol @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; +import { LegacyVaultHandler } from "../handlers/LegacyVaultHandler.sol"; + +/// @notice Stateful invariants over the LEGACY withdrawal path — the +/// pre-upgrade (afterUpgrade=0) requests created via `requestOldWithdrawal`, +/// interleaved with normal deposits / new requests / delegation / executes. +/// This is the code path (beforeUpgrade branch of executeWithdrawal, plus the +/// legacy request that skips `totalPendingWithdrawals`) that no other suite +/// touches. If any sequence lets an attacker inflate votes past their live +/// stake, drain the store, or make the vault insolvent, the fuzzer surfaces it. +/// +/// LG1 solvency: balanceOf(vault) >= Σ user.amount +/// LG3 per-user bound: user.pendingWithdrawals <= user.amount +/// LGV1 vote conservation: Σ currentVotes == Σ (amount − pending) over delegators +/// LGV4 vote solvency: Σ currentVotes <= Σ (amount − pending) [free stake] +/// LGR reward emission cap: store payout <= rewardPerBlock * elapsedBlocks +/// +/// Note: I2 (totalPendingWithdrawals == Σ user.pendingWithdrawals) is +/// intentionally NOT asserted — the legacy request path increments +/// user.pendingWithdrawals without touching totalPendingWithdrawals by design, +/// so the two legitimately diverge once a legacy request exists. +contract LegacyPathInvariants is XVSVaultTestBase { + LegacyVaultHandler internal handler; + uint256 internal startBlock; + + function setUp() public { + _deployAndWire(); + startBlock = block.number; + handler = new LegacyVaultHandler(vault, xvs, actors); + targetContract(address(handler)); + } + + function invariant_LG1_solvency() public view { + assertGe(xvs.balanceOf(address(vault)), _sumAmount(), "LG1: vault under-collateralized"); + } + + function invariant_LG3_pendingWithinAmount() public view { + for (uint256 i = 0; i < actors.length; i++) { + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, actors[i]); + assertLe(pending, amount, "LG3: pending exceeds amount"); + } + } + + function invariant_LGV1_voteConservation() public view { + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "LGV1: votes != delegated live stake"); + } + + function invariant_LGV4_voteSolvency() public view { + uint256 freeStake; + for (uint256 i = 0; i < actors.length; i++) freeStake += _stakeOf(actors[i]); + assertLe(_sumCurrentVotes(), freeStake, "LGV4: votes exceed free stake"); + } + + function invariant_LGR_emissionCap() public view { + uint256 paidOut = STORE_FUNDING - xvs.balanceOf(address(store)); + uint256 maxEmitted = REWARD_PER_BLOCK * (block.number - startBlock); + assertLe(paidOut, maxEmitted, "LGR: store paid out more than emission schedule"); + } + + function invariant_callSummary() public view { + assertTrue( + handler.callDeposit() + + handler.callRequestNew() + + handler.callRequestOld() + + handler.callExecute() + + handler.callDelegate() >= + 0, + "unreachable" + ); + } +} diff --git a/test-fuzz/invariants/RewardSolvency.t.sol b/test-fuzz/invariants/RewardSolvency.t.sol new file mode 100644 index 000000000..4731c4cc3 --- /dev/null +++ b/test-fuzz/invariants/RewardSolvency.t.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; +import { VaultHandler } from "../handlers/VaultHandler.sol"; + +/// @notice Reward-side invariants. Reuses the standard (well-funded store) +/// handler rig and adds two properties the vote/principal suite never touched: +/// +/// R2 emission cap: rewards paid out of the store can never exceed the +/// schedule rewardPerBlock * elapsedBlocks. Since the +/// store holds ONLY the reward token and only ever pays +/// via safeRewardTransfer (principal always moves +/// user<->vault, never through the store), the store's +/// drop equals cumulative rewards paid. A bug in +/// accRewardPerShare that over-mints trips this. +/// R1b noRewardUnderflow: pendingReward() must never revert for any actor. +/// pendingReward mirrors the `.sub(user.rewardDebt)` +/// used across deposit/claim/requestWithdrawal, so a +/// reward-debt underflow (which would brick every user +/// action -> funds locked) surfaces here first. +contract RewardSolvencyInvariants is XVSVaultTestBase { + VaultHandler internal handler; + uint256 internal startBlock; + + function setUp() public { + _deployAndWire(); + startBlock = block.number; + handler = new VaultHandler(vault, xvs, actors); + targetContract(address(handler)); + } + + function invariant_R2_emissionCap() public view { + uint256 paidOut = STORE_FUNDING - xvs.balanceOf(address(store)); + uint256 maxEmitted = REWARD_PER_BLOCK * (block.number - startBlock); + assertLe(paidOut, maxEmitted, "R2: store paid out more than emission schedule"); + } + + function invariant_R1b_noRewardUnderflow() public view { + for (uint256 i = 0; i < actors.length; i++) { + // Reverts here (arithmetic underflow) == reward-debt corruption == + // deposit/claim/requestWithdrawal would revert too -> DoS. + vault.pendingReward(address(xvs), 0, actors[i]); + } + } +} diff --git a/test-fuzz/invariants/XVSVaultInvariants.t.sol b/test-fuzz/invariants/XVSVaultInvariants.t.sol new file mode 100644 index 000000000..b23f78d9e --- /dev/null +++ b/test-fuzz/invariants/XVSVaultInvariants.t.sol @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; +import { VaultHandler } from "../handlers/VaultHandler.sol"; + +/// @notice Stateful invariant suite. The handler drives random sequences of +/// deposit / requestWithdrawal / executeWithdrawal / claim / delegate / +/// warpRoll / donate across a fixed actor set; after every call these +/// properties must hold. A failing counterexample is a real exploit trace. +/// +/// I1 solvency: balanceOf(vault) >= Σ user.amount +/// I2 pending accounting: totalPendingWithdrawals == Σ user.pendingWithdrawals +/// I3 per-user bound: user.pendingWithdrawals <= user.amount +/// V1 vote conservation: Σ currentVotes == Σ (amount − pending) over delegators +/// V4 vote solvency: Σ currentVotes <= balanceOf(vault) − totalPending +contract XVSVaultInvariants is XVSVaultTestBase { + VaultHandler internal handler; + + function setUp() public { + _deployAndWire(); + handler = new VaultHandler(vault, xvs, actors); + targetContract(address(handler)); + } + + function invariant_I1_solvency() public view { + assertGe(xvs.balanceOf(address(vault)), _sumAmount(), "I1: vault under-collateralized"); + } + + function invariant_I2_pendingAccounting() public view { + assertEq(vault.totalPendingWithdrawals(address(xvs), 0), _sumPending(), "I2: pending mismatch"); + } + + function invariant_I3_pendingWithinAmount() public view { + for (uint256 i = 0; i < actors.length; i++) { + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, actors[i]); + assertLe(pending, amount, "I3: pending exceeds amount"); + } + } + + function invariant_V1_voteConservation() public view { + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "V1: votes != delegated stake"); + } + + function invariant_V4_voteSolvency() public view { + uint256 free = xvs.balanceOf(address(vault)) - vault.totalPendingWithdrawals(address(xvs), 0); + assertLe(_sumCurrentVotes(), free, "V4: votes exceed free stake"); + } + + /// @notice Surface handler coverage so a run that never reached deposits + /// (all reverts) is visible rather than a false green. + function invariant_callSummary() public view { + assertTrue( + handler.callDeposit() + handler.callRequest() + handler.callExecute() + handler.callDelegate() >= 0, + "unreachable" + ); + } +} diff --git a/test-fuzz/mocks/MockACM.sol b/test-fuzz/mocks/MockACM.sol new file mode 100644 index 000000000..61ee2f078 --- /dev/null +++ b/test-fuzz/mocks/MockACM.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +/// @notice Always-allow AccessControlManager stand-in. XVSVault's +/// AccessControlledV5._checkAccessAllowed calls only +/// `isAllowedToCall(address,string)`, so this single selector is all that is +/// required. Gating correctness itself is verified on-chain (see the security +/// report); here we simply let the admin wiring/handlers exercise the logic. +contract MockACM { + function isAllowedToCall(address, string calldata) external pure returns (bool) { + return true; + } +} diff --git a/test-fuzz/mocks/MockBEP20.sol b/test-fuzz/mocks/MockBEP20.sol new file mode 100644 index 000000000..0dd57163b --- /dev/null +++ b/test-fuzz/mocks/MockBEP20.sol @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +/// @notice Minimal ERC20/BEP20 used as both the staked token and the reward +/// token for XVSVault pool 0. Voting power in the vault is amount-based and +/// token-agnostic, so a clean mock (no transfer lock, no uint96 packing) +/// faithfully exercises the vault logic while keeping the rig deterministic. +contract MockBEP20 { + string public name = "Mock XVS"; + string public symbol = "mXVS"; + uint8 public constant decimals = 18; + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + mapping(address => mapping(address => uint256)) public allowance; + + event Transfer(address indexed from, address indexed to, uint256 value); + event Approval(address indexed owner, address indexed spender, uint256 value); + + function mint(address to, uint256 amount) external { + totalSupply += amount; + balanceOf[to] += amount; + emit Transfer(address(0), to, amount); + } + + function approve(address spender, uint256 amount) external returns (bool) { + allowance[msg.sender][spender] = amount; + emit Approval(msg.sender, spender, amount); + return true; + } + + function transfer(address to, uint256 amount) external returns (bool) { + _transfer(msg.sender, to, amount); + return true; + } + + function transferFrom(address from, address to, uint256 amount) external returns (bool) { + uint256 allowed = allowance[from][msg.sender]; + if (allowed != type(uint256).max) { + require(allowed >= amount, "MockBEP20: allowance"); + allowance[from][msg.sender] = allowed - amount; + } + _transfer(from, to, amount); + return true; + } + + function _transfer(address from, address to, uint256 amount) internal { + require(balanceOf[from] >= amount, "MockBEP20: balance"); + balanceOf[from] -= amount; + balanceOf[to] += amount; + emit Transfer(from, to, amount); + } +} diff --git a/test-fuzz/scenarios/DelegateBySig.t.sol b/test-fuzz/scenarios/DelegateBySig.t.sol new file mode 100644 index 000000000..b767c55b3 --- /dev/null +++ b/test-fuzz/scenarios/DelegateBySig.t.sol @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice Attacks on the off-chain delegation path (`delegateBySig`). An XVS +/// holder must not be able to move ANOTHER account's votes by replaying, +/// re-timing, cross-chaining, or malleating a signature. The vault recovers the +/// signatory from the EIP-712 digest and delegates THAT account's stake, so a +/// forged/stale signature that recovers to the wrong address must never move the +/// victim's votes. +/// +/// X4 a relayed signature delegates once; the used signature cannot replay +/// (nonce is consumed) +/// X5a an expired signature is rejected +/// X5b a wrong-chainId signature cannot move the signer's votes +/// X5c a malleable (high-s) signature is rejected by ECDSA +/// I11d a forged payload only ever affects the signer, never a victim +contract DelegateBySigTest is XVSVaultTestBase { + bytes32 internal constant DOMAIN_TYPEHASH = + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); + bytes32 internal constant DELEGATION_TYPEHASH = + keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)"); + // secp256k1 curve order n (used to build the malleable counterpart s' = n - s). + uint256 internal constant SECP256K1_N = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141; + + function setUp() public { + _deployAndWire(); + } + + /// Mints, approves and stakes `amt` for the account controlled by `pk`. + function _seedSigner(uint256 pk, uint256 amt) internal returns (address who) { + who = vm.addr(pk); + xvs.mint(who, amt); + vm.prank(who); + xvs.approve(address(vault), type(uint256).max); + vm.prank(who); + vault.deposit(address(xvs), 0, amt); + } + + function _digest( + address delegatee, + uint256 nonce, + uint256 expiry, + uint256 chainId + ) internal view returns (bytes32) { + bytes32 domainSeparator = keccak256( + abi.encode(DOMAIN_TYPEHASH, keccak256(bytes("XVSVault")), chainId, address(vault)) + ); + bytes32 structHash = keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry)); + return keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } + + // ---- X4: a relayed signature delegates once, and replay is rejected ---- + // The call is made by the test contract, not the signer, so this also covers + // the "valid signature relayed by a third party" case. + function test_X4_relayedOnceThenReplayRejected() public { + uint256 pk = 0xA11CE; + address signer = _seedSigner(pk, 50_000e18); + address B = actors[0]; + uint256 expiry = block.timestamp + 1 days; + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, _digest(B, 0, expiry, block.chainid)); + + vault.delegateBySig(B, 0, expiry, v, r, s); + assertEq(uint256(vault.getCurrentVotes(B)), 50_000e18, "X4: first delegate failed"); + assertEq(vault.delegates(signer), B, "X4: delegation not recorded"); + assertEq(vault.nonces(signer), 1, "X4: nonce not consumed"); + + // Same signature again: the nonce is now 1, so it must revert. + vm.expectRevert(bytes("XVSVault::delegateBySig: invalid nonce")); + vault.delegateBySig(B, 0, expiry, v, r, s); + } + + // ---- X5a: expired signature must be rejected ---- + function test_X5_expiredSigRejected() public { + uint256 pk = 0xB0B; + _seedSigner(pk, 50_000e18); + address B = actors[0]; + uint256 expiry = block.timestamp - 1; // already in the past + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, _digest(B, 0, expiry, block.chainid)); + + vm.expectRevert(bytes("XVSVault::delegateBySig: signature expired")); + vault.delegateBySig(B, 0, expiry, v, r, s); + } + + // ---- X5b: wrong-chainId signature cannot move the signer's votes ---- + function test_X5_wrongChainIdCannotMoveVotes() public { + uint256 pk = 0xCA11; + address signer = _seedSigner(pk, 50_000e18); + address B = actors[0]; + uint256 expiry = block.timestamp + 1 days; + + // Sign against a different chainId: recovers some other address (with no + // stake), so it can neither move the signer's votes nor grant B any. + (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, _digest(B, 0, expiry, block.chainid + 1)); + + vault.delegateBySig(B, 0, expiry, v, r, s); + assertEq(uint256(vault.getCurrentVotes(B)), 0, "X5b: cross-chain sig moved votes"); + assertEq(vault.delegates(signer), address(0), "X5b: signer delegation altered"); + } + + // ---- X5c: malleable (high-s) signature must be rejected ---- + function test_X5_malleableSigRejected() public { + uint256 pk = 0xDEAD; + _seedSigner(pk, 50_000e18); + address B = actors[0]; + uint256 expiry = block.timestamp + 1 days; + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, _digest(B, 0, expiry, block.chainid)); + + // Flip to the malleable counterpart: s' = n - s, v' = 27<->28. + bytes32 sMal = bytes32(SECP256K1_N - uint256(s)); + uint8 vMal = v == 27 ? 28 : 27; + + vm.expectRevert(bytes("ECDSA: invalid signature 's' value")); + vault.delegateBySig(B, 0, expiry, vMal, r, sMal); + } + + // ---- I11d: a relayer cannot forge a victim's delegation ---- + function test_I11d_noVictimForge() public { + // Victim stakes and self-delegates through the normal path. + address victim = _seedSigner(0x71C7, 50_000e18); + vm.prank(victim); + vault.delegate(victim); + assertEq(uint256(vault.getCurrentVotes(victim)), 50_000e18, "I11d: setup"); + + // Attacker signs with ITS OWN key, trying to redirect votes to itself. + uint256 attackerPk = 0xBADBAD; + address attacker = vm.addr(attackerPk); + uint256 expiry = block.timestamp + 1 hours; + uint256 n = vault.nonces(attacker); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(attackerPk, _digest(attacker, n, expiry, block.chainid)); + vault.delegateBySig(attacker, n, expiry, v, r, s); + + // The signature only affected the attacker (zero stake). Victim untouched. + assertEq(vault.delegates(victim), victim, "I11d: victim delegate changed"); + assertEq(uint256(vault.getCurrentVotes(victim)), 50_000e18, "I11d: victim votes moved"); + assertEq(uint256(vault.getCurrentVotes(attacker)), 0, "I11d: forged votes for attacker"); + } +} diff --git a/test-fuzz/scenarios/LegacyPath.t.sol b/test-fuzz/scenarios/LegacyPath.t.sol new file mode 100644 index 000000000..85a385f7d --- /dev/null +++ b/test-fuzz/scenarios/LegacyPath.t.sol @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice Deterministic probes of the legacy (pre-upgrade, afterUpgrade=0) +/// withdrawal path — the branch of `executeWithdrawal` that pays reward on the +/// full `user.amount` and does not call `_moveDelegates`. This branch is live +/// mainnet bytecode: any user still holding a genuine pre-upgrade request hits +/// it on execute. `requestOldWithdrawal` (scenario-only) is just the setup tool +/// used to reach that real state, the way `deal`/`prank` set up other tests. +/// +/// LG1/LG2 assert the branch keeps votes == live stake and reward bounded. +/// LG_GUARD tests a real production guard: `requestWithdrawal`/`deposit` revert +/// while a beforeUpgrade request is pending, so a user can only ever hold one +/// request type at a time (which is what keeps the branch's accounting sound). +contract LegacyPathTest is XVSVaultTestBase { + address internal A; + address internal B; + + function setUp() public { + _deployAndWire(); + A = actors[0]; + B = actors[1]; + } + + function _dep(address who, uint256 amt) internal { + vm.prank(who); + vault.deposit(address(xvs), 0, amt); + } + + function _warpPastLock() internal { + vm.warp(block.timestamp + LOCK_PERIOD + 1); + vm.roll(block.number + 1); + } + + // ---- LG1: legacy request burns votes; execute preserves votes == stake ---- + function testFuzz_LG1_legacyVoteConsistency(uint256 x, uint256 p) public { + x = bound(x, 2, ACTOR_SEED); + p = bound(p, 1, x - 1); + + _dep(A, x); + vm.prank(A); + vault.delegate(A); + assertEq(uint256(vault.getCurrentVotes(A)), x, "LG1: votes != stake after delegate"); + + // Legacy request burns votes for the requested slice. + vm.prank(A); + vault.requestOldWithdrawal(address(xvs), 0, p); + assertEq(uint256(vault.getCurrentVotes(A)), x - p, "LG1: legacy request did not burn votes"); + + // Execute the legacy request: amount and pending both drop by p, votes untouched. + _warpPastLock(); + vm.prank(A); + vault.executeWithdrawal(address(xvs), 0); + + // votes (x-p) must still equal live stake (amount-pending) = (x-p)-0. + assertEq(uint256(vault.getCurrentVotes(A)), x - p, "LG1: execute altered votes"); + assertEq(_stakeOf(A), x - p, "LG1: stake != votes after legacy execute"); + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "LG1: conservation broken"); + } + + // ---- LG2: legacy execute reward is bounded by the emission schedule ---- + function testFuzz_LG2_legacyRewardBounded(uint256 x, uint256 dt) public { + x = bound(x, 1e18, ACTOR_SEED); + dt = bound(dt, 1 days, 20 days); + uint256 storeBefore = xvs.balanceOf(address(store)); + + _dep(A, x); + vm.prank(A); + vault.requestOldWithdrawal(address(xvs), 0, x); // whole stake, legacy + vm.roll(block.number + 200_000); + vm.warp(block.timestamp + dt + LOCK_PERIOD + 1); + + vm.prank(A); + vault.executeWithdrawal(address(xvs), 0); + + // Store can never pay out more than the whole store; no wrap/mint. + assertLe(storeBefore - xvs.balanceOf(address(store)), storeBefore, "LG2: store over-drained via legacy path"); + // Principal returned exactly (no inflation). + assertEq(_amountOf(A), 0, "LG2: principal not fully withdrawn"); + } + + // ---- LG_GUARD: production guard blocks the dangerous mixed state ---- + // A user with a beforeUpgrade request pending CANNOT create a new request — + // this is why mainnet can never reach the frozen mixed state. + function test_LG_GUARD_newRequestBlockedWhileLegacyPending() public { + _dep(A, 1_000e18); + vm.prank(A); + vault.requestOldWithdrawal(address(xvs), 0, 400e18); + + vm.prank(A); + vm.expectRevert(bytes("execute pending withdrawal")); + vault.requestWithdrawal(address(xvs), 0, 100e18); + + // deposit is blocked too. + vm.prank(A); + vm.expectRevert(bytes("execute pending withdrawal")); + vault.deposit(address(xvs), 0, 100e18); + } +} diff --git a/test-fuzz/scenarios/MultiPool.t.sol b/test-fuzz/scenarios/MultiPool.t.sol new file mode 100644 index 000000000..0020439a6 --- /dev/null +++ b/test-fuzz/scenarios/MultiPool.t.sol @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; +import { MockBEP20 } from "../mocks/MockBEP20.sol"; + +/// @notice M1/M2 — multi-pool isolation. The live vault runs a single pool, but +/// the code supports many pools under one reward token. Adds pool 1 (reward +/// token = XVS, staked token = a DIFFERENT token xvs2) alongside pool 0 +/// (XVS/XVS) and checks the two properties a second pool must not violate: +/// +/// M2 votes come ONLY from the XVS-staked pool: getStakeAmount iterates +/// poolInfos[xvsAddress] and returns the pool whose token == xvsAddress +/// (pool 0). Staking xvs2 in pool 1 must grant ZERO voting power even +/// though it shares the XVS reward token — otherwise a second pool would +/// be a vote-inflation backdoor. +/// M1 reward isolation: a pool-1-only staker earns from pool 1 and cannot +/// claim pool 0 (and vice versa); accRewardPerShare is per-pool. +contract MultiPoolTest is XVSVaultTestBase { + MockBEP20 internal xvs2; + address internal A; // pool 0 (XVS) staker + address internal B; // pool 1 (xvs2) staker + + uint256 internal constant PID0 = 0; + uint256 internal constant PID1 = 1; + + function setUp() public { + _deployAndWire(); + A = actors[0]; + B = actors[1]; + + // Second staked token, same reward token (XVS), equal alloc -> 50/50 split. + xvs2 = new MockBEP20(); + vault.add(address(xvs), 100, address(xvs2), REWARD_PER_BLOCK, LOCK_PERIOD); + + for (uint256 i = 0; i < actors.length; i++) { + xvs2.mint(actors[i], ACTOR_SEED); + vm.prank(actors[i]); + xvs2.approve(address(vault), type(uint256).max); + } + } + + function _warp(uint256 dt) internal { + vm.warp(block.timestamp + dt); + vm.roll(block.number + (dt / 3) + 1); + } + + // ---- M2: xvs2 stake in pool 1 grants no votes; pool 0 unaffected ---- + function testFuzz_M2_secondPoolGrantsNoVotes(uint256 y, uint256 x) public { + y = bound(y, 1, ACTOR_SEED); + x = bound(x, 1, ACTOR_SEED); + + // B stakes xvs2 in pool 1 and delegates to itself. + vm.prank(B); + vault.deposit(address(xvs), PID1, y); + vm.prank(B); + vault.delegate(B); + assertEq(uint256(vault.getCurrentVotes(B)), 0, "M2: xvs2 pool granted votes"); + + // A stakes XVS in pool 0 and delegates: votes == pool-0 stake only. + vm.prank(A); + vault.delegate(A); + vm.prank(A); + vault.deposit(address(xvs), PID0, x); + assertEq(uint256(vault.getCurrentVotes(A)), x, "M2: pool-0 votes wrong"); + + // A also stakes xvs2 in pool 1: must NOT change A's votes. + vm.prank(A); + vault.deposit(address(xvs), PID1, y); + assertEq(uint256(vault.getCurrentVotes(A)), x, "M2: pool-1 stake leaked into votes"); + } + + // ---- M1: rewards are isolated per pool ---- + function testFuzz_M1_rewardIsolation(uint256 x, uint256 y, uint256 dt) public { + x = bound(x, 1e18, ACTOR_SEED); + y = bound(y, 1e18, ACTOR_SEED); + dt = bound(dt, 1 days, 30 days); + + vm.prank(A); + vault.deposit(address(xvs), PID0, x); // pool 0 only + vm.prank(B); + vault.deposit(address(xvs), PID1, y); // pool 1 only + _warp(dt); + + // Each earns in its own pool. + assertGt(vault.pendingReward(address(xvs), PID0, A), 0, "M1: pool-0 staker earned nothing"); + assertGt(vault.pendingReward(address(xvs), PID1, B), 0, "M1: pool-1 staker earned nothing"); + + // Neither has any stake (or reward) in the other's pool. + assertEq(vault.pendingReward(address(xvs), PID1, A), 0, "M1: A leaked into pool 1"); + assertEq(vault.pendingReward(address(xvs), PID0, B), 0, "M1: B leaked into pool 0"); + + // Claiming the foreign pool pays nothing (no revert, no cross-drain). + uint256 balA = xvs.balanceOf(A); + vm.prank(A); + vault.claim(A, address(xvs), PID1); + assertEq(xvs.balanceOf(A), balA, "M1: A drained reward from pool 1"); + } +} diff --git a/test-fuzz/scenarios/RewardDebt.t.sol b/test-fuzz/scenarios/RewardDebt.t.sol new file mode 100644 index 000000000..b67a67570 --- /dev/null +++ b/test-fuzz/scenarios/RewardDebt.t.sol @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice R1 — reward-debt integrity across the withdrawal lifecycle. The +/// dangerous seam: executeWithdrawal's afterUpgrade branch decrements +/// `user.amount` but does NOT recompute `user.rewardDebt`, while +/// requestWithdrawal recomputes rewardDebt AFTER pending grew and deposit/claim +/// recompute against (amount - pending). If any interleaving leaves +/// `rewardDebt > (amount - pending) * accRewardPerShare / 1e12`, then +/// _computeReward's `.sub(rewardDebt)` underflows and reverts — and because +/// deposit / claim / requestWithdrawal all call it, a single corrupted user +/// bricks their own funds (permanent DoS). These sequences hammer the seam +/// with reward accrual (warps) between every step and assert nothing reverts. +contract RewardDebtTest is XVSVaultTestBase { + address internal A; + address internal B; + + function setUp() public { + _deployAndWire(); + A = actors[0]; + B = actors[1]; + } + + function _dep(address who, uint256 amt) internal { + vm.prank(who); + vault.deposit(address(xvs), 0, amt); + } + + function _req(address who, uint256 amt) internal { + vm.prank(who); + vault.requestWithdrawal(address(xvs), 0, amt); + } + + function _warp(uint256 dt) internal { + vm.warp(block.timestamp + dt); + vm.roll(block.number + (dt / 3) + 1); + } + + /// After every reachable state the reward path must stay callable. + function _assertRewardPathLive(address who) internal { + // pendingReward mirrors the same .sub(rewardDebt); must not underflow. + vault.pendingReward(address(xvs), 0, who); + // A real claim must not revert either (it is the actual user action). + vm.prank(who); + vault.claim(who, address(xvs), 0); + } + + // ---- R1a: request -> wait -> execute -> claim, with accrual between ---- + function testFuzz_R1a_executeThenClaim(uint256 x, uint256 p, uint256 dt1, uint256 dt2) public { + x = bound(x, 2, ACTOR_SEED); + p = bound(p, 1, x - 1); + dt1 = bound(dt1, 1, 30 days); + dt2 = bound(dt2, 1, 30 days); + + _dep(A, x); + _warp(dt1); + _req(A, p); + // clear the 7-day lock, then execute the afterUpgrade request + _warp(LOCK_PERIOD + dt2); + vm.prank(A); + vault.executeWithdrawal(address(xvs), 0); + + _warp(dt1); + _assertRewardPathLive(A); + + // remaining principal is still (x - p) and reward path survives a re-deposit + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, A); + assertEq(amount - pending, x - p, "R1a: principal drift after execute"); + } + + // ---- R1b: partial-request storm — many overlapping requests + claims ---- + function testFuzz_R1b_partialRequestStorm(uint256 x, uint256 seed) public { + x = bound(x, 10, ACTOR_SEED); + _dep(A, x); + + uint256 remaining = x; + for (uint256 i = 0; i < 5; i++) { + _warp(bound(uint256(keccak256(abi.encode(seed, i))), 1, 20 days)); + (uint256 amount, , uint256 pending) = vault.getUserInfo(address(xvs), 0, A); + uint256 avail = amount - pending; + if (avail == 0) break; + uint256 req = bound(uint256(keccak256(abi.encode(seed, i, "r"))), 1, avail); + _req(A, req); + remaining -= req; + _assertRewardPathLive(A); + } + // no underflow / no revert reached here == reward-debt stayed consistent + assertLe(remaining, x, "R1b: sanity"); + } + + // ---- R1c: second staker joins mid-stream; accRewardPerShare moves under A ---- + function testFuzz_R1c_secondStaker(uint256 x, uint256 y, uint256 dt) public { + x = bound(x, 2, ACTOR_SEED); + y = bound(y, 1, ACTOR_SEED); + dt = bound(dt, 1, 30 days); + + _dep(A, x); + _warp(dt); + _dep(B, y); // changes supply -> accRewardPerShare denominator shifts + _warp(dt); + _req(A, bound(x, 1, x)); + _assertRewardPathLive(A); + _assertRewardPathLive(B); + } +} diff --git a/test-fuzz/scenarios/RewardIntegrity.t.sol b/test-fuzz/scenarios/RewardIntegrity.t.sol new file mode 100644 index 000000000..f17982fae --- /dev/null +++ b/test-fuzz/scenarios/RewardIntegrity.t.sol @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice Reward-path attacks by an XVS holder. Rewards are paid from the +/// separate XVSStore; the goal is to prove a holder cannot mint reward from +/// nothing, redirect another user's reward to themselves, or double-collect via +/// the deferred-debt (`pendingRewardTransfers`) path when the store is +/// underfunded. +/// +/// X3a a second claim in the same block yields nothing (no double-collect) +/// X3b claiming for another account pays THAT account, not the caller +/// X6 the vault-debt path repays exactly the owed amount, never more +/// X9 a pending (requested) slice stops earning reward +contract RewardIntegrityTest is XVSVaultTestBase { + function setUp() public { + _deployAndWire(); + } + + // ---- X3a: double claim in one block pays only once ---- + function test_X3_doubleClaimYieldsNothing() public { + address A = actors[0]; + vm.prank(A); + vault.deposit(address(xvs), 0, 50_000e18); + + vm.roll(block.number + 1000); + vm.warp(block.timestamp + 3000); + + uint256 b0 = xvs.balanceOf(A); + vm.prank(A); + vault.claim(A, address(xvs), 0); + uint256 gained = xvs.balanceOf(A) - b0; + assertGt(gained, 0, "X3a: no reward accrued"); + + // Same block, second claim: rewardDebt is settled, so nothing more. + vm.prank(A); + vault.claim(A, address(xvs), 0); + assertEq(xvs.balanceOf(A), b0 + gained, "X3a: double-claim paid twice"); + } + + // ---- X3b: claim(account) credits the account, not the caller ---- + function test_X3_claimForOtherPaysOther() public { + address A = actors[0]; + address attacker = actors[1]; + vm.prank(A); + vault.deposit(address(xvs), 0, 50_000e18); + + vm.roll(block.number + 1000); + vm.warp(block.timestamp + 3000); + + uint256 attackerBefore = xvs.balanceOf(attacker); + uint256 aBefore = xvs.balanceOf(A); + + // Attacker triggers A's claim; funds must flow to A. + vm.prank(attacker); + vault.claim(A, address(xvs), 0); + + assertEq(xvs.balanceOf(attacker), attackerBefore, "X3b: attacker skimmed reward"); + assertGt(xvs.balanceOf(A), aBefore, "X3b: rightful owner not paid"); + } + + // ---- X6: underfunded store records debt and repays it exactly once ---- + function test_X6_vaultDebtNoDoublePay() public { + address A = actors[0]; + vm.prank(A); + vault.deposit(address(xvs), 0, 50_000e18); + + // Warp far enough that accrued reward exceeds the store balance. + vm.roll(block.number + 2_000_000); + vm.warp(block.timestamp + 6_000_000); + + uint256 storeBal = xvs.balanceOf(address(store)); + uint256 aBefore = xvs.balanceOf(A); + + vm.prank(A); + vault.claim(A, address(xvs), 0); + uint256 firstPay = xvs.balanceOf(A) - aBefore; + uint256 debt = vault.pendingRewardTransfers(address(xvs), A); + + // Store is drained to zero and the shortfall is booked as debt. + assertEq(firstPay, storeBal, "X6: partial pay != store balance"); + assertGt(debt, 0, "X6: expected recorded debt"); + assertEq(xvs.balanceOf(address(store)), 0, "X6: store not fully drained"); + + // Accrue a little more, refund the store enough to cover debt + new + // pending (with buffer), claim again. + vm.roll(block.number + 10); + vm.warp(block.timestamp + 30); + xvs.mint(address(store), debt + 1_000_000e18); + + uint256 p = vault.pendingReward(address(xvs), 0, A); // fresh reward at this block + uint256 mid = xvs.balanceOf(A); + vm.prank(A); + vault.claim(A, address(xvs), 0); + uint256 secondPay = xvs.balanceOf(A) - mid; + + // Repay is exactly the new pending plus the booked debt — never more. + assertEq(secondPay, p + debt, "X6: repay != pending + debt"); + assertEq(vault.pendingRewardTransfers(address(xvs), A), 0, "X6: debt not cleared"); + } + + // ---- X9: a requested (pending) slice earns no further reward ---- + function test_X9_pendingSliceEarnsNoReward() public { + address A = actors[0]; + address B = actors[1]; + vm.prank(A); + vault.deposit(address(xvs), 0, 100_000e18); + vm.prank(B); + vault.deposit(address(xvs), 0, 100_000e18); + + // A requests withdrawal of half -> that slice stops accruing. + vm.prank(A); + vault.requestWithdrawal(address(xvs), 0, 50_000e18); + + vm.roll(block.number + 1000); + vm.warp(block.timestamp + 3000); + + uint256 ra = vault.pendingReward(address(xvs), 0, A); // earns on 50k + uint256 rb = vault.pendingReward(address(xvs), 0, B); // earns on 100k + + assertGt(ra, 0, "X9: active slice earned nothing"); + assertGt(rb, ra, "X9: pending slice still earned reward"); + } +} diff --git a/test-fuzz/scenarios/Solc0516Hacks.t.sol b/test-fuzz/scenarios/Solc0516Hacks.t.sol new file mode 100644 index 000000000..b5e8f52c4 --- /dev/null +++ b/test-fuzz/scenarios/Solc0516Hacks.t.sol @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice PoC exploit attempts for the canonical pre-0.8 / Solidity-0.5.x hack +/// classes, run against the real 0.5.16 `XVSVault`. Each test *performs* the +/// attack and asserts it is blocked — a documented failed exploit is the proof +/// the mitigation holds. Classes covered (see the security review): +/// +/// H1 integer underflow — pre-0.8 subtraction wraps to a huge number; here +/// SafeMath / the amount guard revert instead. +/// H2 reward-debt wrap — the money version of H1: if reward math could +/// underflow, pending reward wraps to ~2^256 and +/// drains the store. SafeMath `.sub` reverts. +/// (H3 sig-malleability replay lives in DelegateBySig.t.sol::X5c and the live +/// ForkLiveHacks suite; not duplicated here.) +/// H4 ecrecover(0) forge — a garbage signature must not recover to a usable +/// signatory (never address(0) with a live nonce). +/// H5 checkpoint double-vote — the historical Compound/Venus bug (the +/// `__old*Slot` DEPRECATED storage): keep votes after +/// moving the underlying XVS to a second wallet. +contract Solc0516HacksTest is XVSVaultTestBase { + address internal attacker; + + function setUp() public { + _deployAndWire(); + attacker = actors[0]; + } + + // ---- H1: withdraw more than staked must revert, not underflow ---- + // Pre-0.8, `user.amount - amount` with amount > user.amount wraps to ~2^256, + // making the vault think the attacker holds astronomical principal. + function test_H1_withdrawOverStakeReverts() public { + vm.prank(attacker); + vault.deposit(address(xvs), 0, 1_000e18); + + // Request 1 wei more than staked. + vm.prank(attacker); + vm.expectRevert(bytes("requested amount is invalid")); + vault.requestWithdrawal(address(xvs), 0, 1_000e18 + 1); + + // Principal is untouched — no wrap happened. + (uint256 amount, , ) = vault.getUserInfo(address(xvs), 0, attacker); + assertEq(amount, 1_000e18, "H1: stake corrupted"); + } + + // ---- H2: reward accounting cannot underflow into an infinite mint ---- + // The pre-0.8 jackpot: drive rewardDebt above cumulative so `.sub` wraps and + // pendingReward becomes ~2^256, then claim to drain the store. SafeMath makes + // every such path revert; here we show the store can never be over-drained + // and pendingReward stays sane through an adversarial deposit/withdraw churn. + function test_H2_noRewardUnderflowMint() public { + uint256 storeBefore = xvs.balanceOf(address(store)); + + vm.prank(attacker); + vault.deposit(address(xvs), 0, 500e18); + vm.warp(block.timestamp + 30 days); + vm.roll(block.number + 1); + + // Churn: request part, wait, execute, re-deposit — the reward-debt seam. + vm.prank(attacker); + vault.requestWithdrawal(address(xvs), 0, 200e18); + vm.warp(block.timestamp + LOCK_PERIOD + 1); + vm.roll(block.number + 1); + vm.prank(attacker); + vault.executeWithdrawal(address(xvs), 0); + + // pendingReward must not be an astronomical (wrapped) number. + uint256 pending = vault.pendingReward(address(xvs), 0, attacker); + assertLt(pending, storeBefore, "H2: pending reward exceeds entire store (wrap?)"); + + // Claim cannot pull more reward than the store ever held. + vm.prank(attacker); + vault.claim(attacker, address(xvs), 0); + assertGe(xvs.balanceOf(address(store)), 0, "H2: store went negative"); + // Reward paid out (store delta) is bounded by the emission schedule, not 2^256. + assertLe(storeBefore - xvs.balanceOf(address(store)), storeBefore, "H2: store over-drained"); + } + + // NOTE: H3 (signature-malleability replay) lived here but is an exact + // duplicate of DelegateBySig.t.sol::test_X5_malleableSigRejected (local) and + // ForkLiveHacks::test_fork_H3 (live). Removed to avoid redundant coverage. + + // ---- H4: forged signature cannot move real votes to a chosen address ---- + // A well-formed but attacker-fabricated sig (low-s, v=27) does NOT revert and + // does NOT recover to address(0): ecrecover returns a deterministic address + // the attacker cannot choose and does not control. The vault delegates THAT + // phantom's stake — which is zero. The security property is therefore not + // "it reverts" but "a forged sig can never grant the attacker's target any + // votes", because forging a sig that recovers to a funded victim requires + // that victim's private key. + function test_H4_ecrecoverForgeGrantsNoVotes() public { + address target = actors[1]; // address the attacker WANTS to empower + uint256 expiry = block.timestamp + 1 hours; + + // s=1 is in the lower half order and v=27 is valid, so the lib accepts the + // shape and recovers some phantom signatory (with no stake, nonce 0). + try vault.delegateBySig(target, 0, expiry, 27, bytes32(uint256(1)), bytes32(uint256(1))) { + // No revert: the phantom had zero stake, so nothing moved. + } catch { + // Reverting is equally acceptable. + } + + // Either way, the forged signature granted the target no voting power. + assertEq(uint256(vault.getCurrentVotes(target)), 0, "H4: forged sig moved votes to attacker's target"); + } + + // ---- H5: historical Compound/Venus double-vote (the DEPRECATED-slot bug) ---- + // Pre-fix, votes tracked balanceOf, so an attacker could delegate, then MOVE + // the underlying XVS to a fresh wallet and have BOTH wallets vote the same + // coins (2x inflation). The fixed vault burns votes at requestWithdrawal and + // ties votes to live *staked* amount, so total votes never exceed staked XVS. + function test_H5_doubleVoteViaTransferBlocked() public { + address w2 = actors[2]; + uint256 x = 50_000e18; + + // Wallet 1 stakes and self-delegates -> x votes. + vm.prank(attacker); + vault.deposit(address(xvs), 0, x); + vm.prank(attacker); + vault.delegate(attacker); + assertEq(uint256(vault.getCurrentVotes(attacker)), x, "H5: setup"); + + // Attempt the "reuse the same coins" move: withdraw and hand them to w2. + vm.prank(attacker); + vault.requestWithdrawal(address(xvs), 0, x); + // Votes are burned the instant withdrawal is requested — not after execute. + assertEq(uint256(vault.getCurrentVotes(attacker)), 0, "H5: votes survived the request (double-count!)"); + + vm.warp(block.timestamp + LOCK_PERIOD + 1); + vm.roll(block.number + 1); + vm.prank(attacker); + vault.executeWithdrawal(address(xvs), 0); + + vm.prank(attacker); + xvs.transfer(w2, x); + vm.prank(w2); + xvs.approve(address(vault), type(uint256).max); + vm.prank(w2); + vault.deposit(address(xvs), 0, x); + vm.prank(w2); + vault.delegate(w2); + + // The coins now back w2's votes ONLY. Total across both wallets == x, not 2x. + assertEq(uint256(vault.getCurrentVotes(w2)), x, "H5: w2 votes wrong"); + assertEq( + uint256(vault.getCurrentVotes(attacker)) + uint256(vault.getCurrentVotes(w2)), + x, + "H5: total votes exceed staked XVS (double-vote succeeded)" + ); + } +} diff --git a/test-fuzz/scenarios/VoteInflation.t.sol b/test-fuzz/scenarios/VoteInflation.t.sol new file mode 100644 index 000000000..e3396dd29 --- /dev/null +++ b/test-fuzz/scenarios/VoteInflation.t.sol @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice Targeted attacks on the vote-accounting seams — the ways 300k XVS +/// could become >300k voting power. The vault moves votes two ways: deposit / +/// requestWithdrawal use an incremental DELTA (_moveDelegates(·,·,amount)), +/// while delegate() moves the ABSOLUTE getStakeAmount() between reps. +/// Interleaving the two is the classic Compound-style inflation source. +/// Each test asserts the conservation law: Σ currentVotes == Σ (stake) over +/// delegators, and — for the governance-relevant case — that a historical +/// snapshot can never exceed the stake that existed at that block. +contract VoteInflationTest is XVSVaultTestBase { + address internal A; + address internal B; // delegatee + address internal C; // delegatee + + function setUp() public { + _deployAndWire(); + A = actors[0]; + B = actors[1]; + C = actors[2]; + } + + function _dep(address who, uint256 amt) internal { + vm.prank(who); + vault.deposit(address(xvs), 0, amt); + } + + function _del(address who, address to) internal { + vm.prank(who); + vault.delegate(to); + } + + function _req(address who, uint256 amt) internal { + vm.prank(who); + vault.requestWithdrawal(address(xvs), 0, amt); + } + + // ---- S1: delta/absolute seam — re-delegate after a partial withdrawal ---- + // delegate(A→B); deposit X; request p; delegate(A→C). + // Expect: votes(B)==0, votes(C)==X−p, and the global conservation holds. + function testFuzz_S1_reDelegateAfterRequest(uint256 x, uint256 p) public { + x = bound(x, 2, ACTOR_SEED); + p = bound(p, 1, x - 1); + + _del(A, B); + _dep(A, x); + assertEq(uint256(vault.getCurrentVotes(B)), x, "S1: B != X after deposit"); + + _req(A, p); + assertEq(uint256(vault.getCurrentVotes(B)), x - p, "S1: B != X-p after request"); + + _del(A, C); + assertEq(uint256(vault.getCurrentVotes(B)), 0, "S1: B not drained on re-delegate"); + assertEq(uint256(vault.getCurrentVotes(C)), x - p, "S1: C != X-p"); + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "S1: conservation broken"); + } + + // ---- S2: re-delegate to the SAME delegatee must net zero ---- + function testFuzz_S2_reDelegateSameTarget(uint256 x, uint8 k) public { + x = bound(x, 1, ACTOR_SEED); + _del(A, B); + _dep(A, x); + uint256 kk = bound(k, 1, 10); + for (uint256 i = 0; i < kk; i++) _del(A, B); + assertEq(uint256(vault.getCurrentVotes(B)), x, "S2: same-target re-delegate inflated"); + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "S2: conservation broken"); + } + + // ---- S3: deposits while undelegated grant 0 votes; one delegate == stake ---- + function testFuzz_S3_depositThenDelegateOnce(uint256 x1, uint256 x2) public { + x1 = bound(x1, 1, ACTOR_SEED / 2); + x2 = bound(x2, 1, ACTOR_SEED / 2); + _dep(A, x1); // undelegated -> no votes + _dep(A, x2); + assertEq(uint256(vault.getCurrentVotes(A)), 0, "S3: votes before delegate"); + _del(A, A); + assertEq(uint256(vault.getCurrentVotes(A)), x1 + x2, "S3: not exactly stake (double-count?)"); + } + + // ---- S4: same-block op storm; checkpoint overwrite must not sum ---- + function testFuzz_S4_sameBlockStorm(uint256 x, uint256 y, uint256 p) public { + x = bound(x, 2, ACTOR_SEED / 2); + y = bound(y, 2, ACTOR_SEED / 2); + p = bound(p, 1, x - 1); + // No warp/roll between any call: all land in the same block. + _dep(A, x); + _del(A, B); + _dep(A, y); + _del(A, C); + _req(A, p); + _del(A, B); + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "S4: conservation broken same-block"); + // B should hold A's whole current stake (last delegate target). + assertEq(uint256(vault.getCurrentVotes(B)), _stakeOf(A), "S4: B != A stake"); + assertEq(uint256(vault.getCurrentVotes(C)), 0, "S4: stale votes on C"); + } + + // ---- S5: historical snapshot can never exceed stake at that block ---- + // The governance-relevant property: getPriorVotes summed over all accounts + // at any past block b <= total staked at b. Attempts cross-wallet reuse of + // the same economic XVS around the snapshot block. + function testFuzz_S5_snapshotNeverExceedsStake(uint256 x, uint256 moveAmt) public { + x = bound(x, 2, ACTOR_SEED); + moveAmt = bound(moveAmt, 1, x - 1); + + // A stakes and delegates; record the snapshot block. + _dep(A, x); + _del(A, A); + vm.roll(block.number + 1); + vm.warp(block.timestamp + 3); + uint256 snap = block.number - 1; // a settled past block + uint256 stakeAtSnap = _sumAmount() - _sumPending(); + + // Attempt to "reuse" the XVS: request withdrawal, wait out the lock, + // execute, move tokens to B, B stakes+delegates. The snapshot at `snap` + // must not retroactively gain B's later votes. + _req(A, moveAmt); + vm.warp(block.timestamp + LOCK_PERIOD + 1); + vm.roll(block.number + 1); + vm.prank(A); + vault.executeWithdrawal(address(xvs), 0); + vm.prank(A); + xvs.transfer(B, moveAmt); + _dep(B, moveAmt); + _del(B, B); + vm.roll(block.number + 1); + vm.warp(block.timestamp + 3); + + uint256 priorSum; + for (uint256 i = 0; i < actors.length; i++) { + priorSum += vault.getPriorVotes(actors[i], snap); + } + assertLe(priorSum, stakeAtSnap, "S5: historical votes exceed stake at snapshot"); + } + + // ---- S6: execute must not re-burn or re-add votes ---- + function testFuzz_S6_executeVoteNeutral(uint256 x, uint256 p, uint256 y) public { + x = bound(x, 2, ACTOR_SEED / 2); + p = bound(p, 1, x); + y = bound(y, 1, ACTOR_SEED / 2); + + _del(A, A); + _dep(A, x); + _req(A, p); + uint256 votesAfterReq = vault.getCurrentVotes(A); + assertEq(votesAfterReq, x - p, "S6: request burn wrong"); + + vm.warp(block.timestamp + LOCK_PERIOD + 1); + vm.roll(block.number + 1); + vm.prank(A); + vault.executeWithdrawal(address(xvs), 0); + // Execute changes principal but must NOT touch votes. + assertEq(uint256(vault.getCurrentVotes(A)), x - p, "S6: execute altered votes"); + + // A later deposit adds exactly y, not more (no re-add of burned votes). + _dep(A, y); + assertEq(uint256(vault.getCurrentVotes(A)), x - p + y, "S6: deposit re-added stale votes"); + assertEq(_sumCurrentVotes(), _sumDelegatedStake(), "S6: conservation broken"); + } +} diff --git a/test-fuzz/scenarios/VoteOverflow.t.sol b/test-fuzz/scenarios/VoteOverflow.t.sol new file mode 100644 index 000000000..57fe03564 --- /dev/null +++ b/test-fuzz/scenarios/VoteOverflow.t.sol @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.8.25; + +import { XVSVaultTestBase } from "../XVSVaultTestBase.sol"; + +/// @notice The attackers minted XVS, so they may hold balances near or above the +/// uint96 vote cap (2^96). Votes are packed into uint96 checkpoints; the vault +/// must never let a stake overflow that width — either a single oversized +/// deposit, or an accumulated sub-cap stake that would overflow only when +/// delegated. Both paths must revert rather than wrap. +/// +/// X7a a deposit of >= 2^96 reverts on the vote-move overflow guard +/// X7b accumulating stake >= 2^96 (sub-cap deposits) then delegating reverts +contract VoteOverflowTest is XVSVaultTestBase { + uint256 internal constant TWO_96 = 2 ** 96; + + function setUp() public { + _deployAndWire(); + } + + // ---- X7a: single deposit at/above the cap reverts ---- + function test_X7_depositAtVoteCapReverts() public { + address A = actors[0]; + xvs.mint(A, TWO_96); // approval is already max from the base wiring + + vm.prank(A); + vm.expectRevert(bytes("XVSVault::deposit: votes overflow")); + vault.deposit(address(xvs), 0, TWO_96); + } + + // ---- X7b: accumulate >= 2^96 undelegated, then delegate reverts ---- + function test_X7_delegateAboveCapReverts() public { + address A = actors[0]; + xvs.mint(A, TWO_96 * 2); + + // Sub-cap deposits succeed while undelegated (no checkpoint written). + vm.prank(A); + vault.deposit(address(xvs), 0, TWO_96 - 1); + vm.prank(A); + vault.deposit(address(xvs), 0, TWO_96 - 1); + + // Total stake ~ 2^97: delegating would push votes past the uint96 cap. + vm.prank(A); + vm.expectRevert(bytes("XVSVault::getStakeAmount: votes overflow")); + vault.delegate(A); + } +}