diff --git a/simulations/vip-583/abi/ProxyAdmin.json b/simulations/vip-583/abi/ProxyAdmin.json new file mode 100644 index 000000000..b4c51d8df --- /dev/null +++ b/simulations/vip-583/abi/ProxyAdmin.json @@ -0,0 +1,151 @@ +[ + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "previousOwner", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "OwnershipTransferred", + "type": "event" + }, + { + "inputs": [ + { + "internalType": "contract TransparentUpgradeableProxy", + "name": "proxy", + "type": "address" + }, + { + "internalType": "address", + "name": "newAdmin", + "type": "address" + } + ], + "name": "changeProxyAdmin", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract TransparentUpgradeableProxy", + "name": "proxy", + "type": "address" + } + ], + "name": "getProxyAdmin", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract TransparentUpgradeableProxy", + "name": "proxy", + "type": "address" + } + ], + "name": "getProxyImplementation", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "owner", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "renounceOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "transferOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract TransparentUpgradeableProxy", + "name": "proxy", + "type": "address" + }, + { + "internalType": "address", + "name": "implementation", + "type": "address" + } + ], + "name": "upgrade", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract TransparentUpgradeableProxy", + "name": "proxy", + "type": "address" + }, + { + "internalType": "address", + "name": "implementation", + "type": "address" + }, + { + "internalType": "bytes", + "name": "data", + "type": "bytes" + } + ], + "name": "upgradeAndCall", + "outputs": [], + "stateMutability": "payable", + "type": "function" + } +] diff --git a/simulations/vip-583/abi/RiskFundV2.json b/simulations/vip-583/abi/RiskFundV2.json new file mode 100644 index 000000000..4371b652d --- /dev/null +++ b/simulations/vip-583/abi/RiskFundV2.json @@ -0,0 +1,626 @@ +[ + { + "inputs": [], + "name": "InsufficientBalance", + "type": "error" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "poolReserve", + "type": "uint256" + } + ], + "name": "InsufficientPoolReserve", + "type": "error" + }, + { + "inputs": [], + "name": "InvalidRiskFundConverter", + "type": "error" + }, + { + "inputs": [], + "name": "InvalidShortfallAddress", + "type": "error" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "sender", + "type": "address" + }, + { + "internalType": "address", + "name": "calledContract", + "type": "address" + }, + { + "internalType": "string", + "name": "methodSignature", + "type": "string" + } + ], + "name": "Unauthorized", + "type": "error" + }, + { + "inputs": [], + "name": "ZeroAddressNotAllowed", + "type": "error" + }, + { + "inputs": [], + "name": "ZeroValueNotAllowed", + "type": "error" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "oldConvertibleBaseAsset", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newConvertibleBaseAsset", + "type": "address" + } + ], + "name": "ConvertibleBaseAssetUpdated", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "uint8", + "name": "version", + "type": "uint8" + } + ], + "name": "Initialized", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address", + "name": "oldAccessControlManager", + "type": "address" + }, + { + "indexed": false, + "internalType": "address", + "name": "newAccessControlManager", + "type": "address" + } + ], + "name": "NewAccessControlManager", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "previousOwner", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "OwnershipTransferStarted", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "previousOwner", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "OwnershipTransferred", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "asset", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "PoolAssetsDecreased", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "asset", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "PoolAssetsIncreased", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "oldRiskFundConverter", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newRiskFundConverter", + "type": "address" + } + ], + "name": "RiskFundConverterUpdated", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "oldShortfallContract", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newShortfallContract", + "type": "address" + } + ], + "name": "ShortfallContractUpdated", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "token", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "to", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "SweepToken", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "token", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "SweepTokenFromPool", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "TransferredReserveForAuction", + "type": "event" + }, + { + "inputs": [], + "name": "acceptOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "accessControlManager", + "outputs": [ + { + "internalType": "contract IAccessControlManagerV8", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "convertibleBaseAsset", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "comptroller", + "type": "address" + } + ], + "name": "getPoolsBaseAssetReserves", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "maxLoopsLimit", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "owner", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "pendingOwner", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + }, + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "poolAssetsFunds", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "renounceOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "riskFundConverter", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "accessControlManager_", + "type": "address" + } + ], + "name": "setAccessControlManager", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "convertibleBaseAsset_", + "type": "address" + } + ], + "name": "setConvertibleBaseAsset", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "riskFundConverter_", + "type": "address" + } + ], + "name": "setRiskFundConverter", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "shortfallContractAddress_", + "type": "address" + } + ], + "name": "setShortfallContractAddress", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "shortfall", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "tokenAddress", + "type": "address" + }, + { + "internalType": "address", + "name": "to", + "type": "address" + }, + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "sweepToken", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "tokenAddress", + "type": "address" + }, + { + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "internalType": "address", + "name": "receiver", + "type": "address" + }, + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "sweepTokenFromPool", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "transferOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "transferReserveForAuction", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "comptroller", + "type": "address" + }, + { + "internalType": "address", + "name": "asset", + "type": "address" + }, + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "updatePoolState", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + } +] diff --git a/simulations/vip-583/bscmainnet.ts b/simulations/vip-583/bscmainnet.ts new file mode 100644 index 000000000..5fc9b3b15 --- /dev/null +++ b/simulations/vip-583/bscmainnet.ts @@ -0,0 +1,233 @@ +import { + SnapshotRestorer, + impersonateAccount, + setBalance, + takeSnapshot, +} from "@nomicfoundation/hardhat-network-helpers"; +import { expect } from "chai"; +import { BigNumber, Contract, Signer } from "ethers"; +import { parseUnits } from "ethers/lib/utils"; +import { ethers } from "hardhat"; +import { forking, testVip } from "src/vip-framework"; + +import { + DEFAULT_PROXY_ADMIN, + RISK_FUND_V2_NEW_IMPLEMENTATION, + RISK_FUND_V2_PROXY, + vip583, +} from "../../vips/vip-583/bscmainnet"; +import PROXY_ADMIN_ABI from "./abi/ProxyAdmin.json"; +import RISK_FUND_V2_ABI from "./abi/RiskFundV2.json"; + +const USDT = "0x55d398326f99059fF775485246999027B3197955"; +const NORMAL_TIMELOCK = "0x939bD8d64c0A9583A7Dcea9933f7b21697ab6396"; + +// Core Pool Comptroller for testing poolAssetsFunds +const CORE_POOL_COMPTROLLER = "0xfD36E2c2a6789Db23113685031d7F16329158384"; + +// USDT whale for donation attack test (Binance Hot Wallet) +const USDT_WHALE = "0x8894E0a0c962CB723c1976a4421c95949bE2D4E3"; + +const ERC20_ABI = [ + "function balanceOf(address) view returns (uint256)", + "function transfer(address to, uint256 amount) returns (bool)", + "function approve(address spender, uint256 amount) returns (bool)", +]; + +forking(73352569, async () => { + let proxyAdmin: Contract; + let riskFundV2: Contract; + let usdt: Contract; + let oldImplementation: string; + let timelockSigner: Signer; + let whaleSigner: Signer; + + // State snapshots for comparison + let preUpgradeState: { + owner: string; + shortfall: string; + riskFundConverter: string; + convertibleBaseAsset: string; + corePoolUsdtReserve: BigNumber; + usdtBalance: BigNumber; + }; + + before(async () => { + proxyAdmin = await ethers.getContractAt(PROXY_ADMIN_ABI, DEFAULT_PROXY_ADMIN); + riskFundV2 = await ethers.getContractAt(RISK_FUND_V2_ABI, RISK_FUND_V2_PROXY); + usdt = await ethers.getContractAt(ERC20_ABI, USDT); + oldImplementation = await proxyAdmin.getProxyImplementation(RISK_FUND_V2_PROXY); + + // Impersonate timelock for later tests + await impersonateAccount(NORMAL_TIMELOCK); + await setBalance(NORMAL_TIMELOCK, parseUnits("100", 18)); + timelockSigner = await ethers.getSigner(NORMAL_TIMELOCK); + + // Impersonate USDT whale for donation attack tests + await impersonateAccount(USDT_WHALE); + await setBalance(USDT_WHALE, parseUnits("100", 18)); + whaleSigner = await ethers.getSigner(USDT_WHALE); + + // Snapshot pre-upgrade state + preUpgradeState = { + owner: await riskFundV2.owner(), + shortfall: await riskFundV2.shortfall(), + riskFundConverter: await riskFundV2.riskFundConverter(), + convertibleBaseAsset: await riskFundV2.convertibleBaseAsset(), + corePoolUsdtReserve: await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT), + usdtBalance: await usdt.balanceOf(RISK_FUND_V2_PROXY), + }; + }); + + describe("Pre-VIP state", async () => { + it("should have the old implementation", async () => { + expect(oldImplementation).to.equal("0x7Ef5ABbcC9A701e728BeB7Afd4fb5747fAB15A28"); + }); + + it("should have correct owner (Normal Timelock)", async () => { + expect(await riskFundV2.owner()).to.equal(NORMAL_TIMELOCK); + }); + + it("should have non-zero USDT balance", async () => { + const balance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + expect(balance).to.be.gt(0); + console.log(` Pre-upgrade USDT balance: ${ethers.utils.formatUnits(balance, 18)} USDT`); + }); + + it("should have tracked pool reserves", async () => { + const corePoolReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + console.log(` Core Pool USDT reserve: ${ethers.utils.formatUnits(corePoolReserve, 18)} USDT`); + }); + + it("BUG REPRODUCTION: donation attack causes sweepToken to revert (DoS)", async () => { + // This test demonstrates the bug being fixed: + // 1. Attacker donates tokens directly to the contract (untracked) + // 2. When governance tries to sweep amount > assetReserves, arithmetic underflow occurs + // + // Bug: preSweepToken used `amount` instead of `amountDiff` in share calculation. + // When amount > assetReserves: distributedShare (based on amount) > amountDiff, + // causing underflow in last pool: poolAmountShare = amountDiff - distributedShare + const snapshot: SnapshotRestorer = await takeSnapshot(); + + try { + // Get current state to calculate proper attack amounts + const corePoolReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + + // Donate enough to make balance significantly larger than tracked reserves + const donationAmount = parseUnits("1000000", 18); // Donate 1M USDT + await usdt.connect(whaleSigner).transfer(RISK_FUND_V2_PROXY, donationAmount); + + const newBalance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + console.log(` Donated ${ethers.utils.formatUnits(donationAmount, 18)} USDT`); + console.log(` New balance: ${ethers.utils.formatUnits(newBalance, 18)} USDT`); + console.log(` Tracked reserves (Core Pool): ${ethers.utils.formatUnits(corePoolReserve, 18)} USDT`); + + // Sweep amount > assetReserves but < balance - this triggers the bug + // When amount > assetReserves: distributedShare calculated with `amount` exceeds amountDiff + const sweepAmount = corePoolReserve.add(parseUnits("100000", 18)); // Tracked + 100k + console.log(` Attempting to sweep: ${ethers.utils.formatUnits(sweepAmount, 18)} USDT`); + + // With buggy code: distributedShare (based on large `amount`) > amountDiff → underflow + await expect(riskFundV2.connect(timelockSigner).sweepToken(USDT, NORMAL_TIMELOCK, sweepAmount)).to.be.reverted; // Arithmetic underflow + + console.log(" ✓ sweepToken reverted as expected (bug confirmed)"); + } finally { + await snapshot.restore(); + } + }); + }); + + testVip("VIP-583 Upgrade RiskFundV2", await vip583()); + + describe("Post-VIP state", async () => { + it("should have the new implementation", async () => { + expect(await proxyAdmin.getProxyImplementation(RISK_FUND_V2_PROXY)).to.equal(RISK_FUND_V2_NEW_IMPLEMENTATION); + }); + + it("should preserve owner after upgrade", async () => { + expect(await riskFundV2.owner()).to.equal(preUpgradeState.owner); + }); + + it("should preserve shortfall contract after upgrade", async () => { + expect(await riskFundV2.shortfall()).to.equal(preUpgradeState.shortfall); + }); + + it("should preserve riskFundConverter after upgrade", async () => { + expect(await riskFundV2.riskFundConverter()).to.equal(preUpgradeState.riskFundConverter); + }); + + it("should preserve convertibleBaseAsset after upgrade", async () => { + expect(await riskFundV2.convertibleBaseAsset()).to.equal(preUpgradeState.convertibleBaseAsset); + }); + + it("should preserve poolAssetsFunds mapping after upgrade", async () => { + const postUpgradeReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + expect(postUpgradeReserve).to.equal(preUpgradeState.corePoolUsdtReserve); + }); + + it("should preserve token balances after upgrade", async () => { + const postUpgradeBalance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + expect(postUpgradeBalance).to.equal(preUpgradeState.usdtBalance); + }); + + it("should allow owner to call sweepToken (basic access check)", async () => { + // This test verifies the function is callable by owner + // We don't actually sweep funds, just verify the call doesn't revert on access + const balance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + + if (balance.gt(0)) { + // Sweep a small amount (1 USDT) to verify functionality + const sweepAmount = parseUnits("1", 18); + if (balance.gte(sweepAmount)) { + await expect(riskFundV2.connect(timelockSigner).sweepToken(USDT, NORMAL_TIMELOCK, sweepAmount)).to.not.be + .reverted; + } + } + }); + + it("should correctly report pool reserves via getPoolsBaseAssetReserves", async () => { + const reserves = await riskFundV2.getPoolsBaseAssetReserves(CORE_POOL_COMPTROLLER); + // Should return the same as poolAssetsFunds for base asset + console.log(` Core Pool base asset reserves: ${ethers.utils.formatUnits(reserves, 18)}`); + }); + + it("FIX VERIFICATION: donation attack no longer causes DoS after upgrade", async () => { + // This test verifies the fix works: + // Same scenario as pre-upgrade bug reproduction, but sweepToken should now succeed + const snapshot: SnapshotRestorer = await takeSnapshot(); + + try { + // Get current state to calculate proper test amounts (same as pre-upgrade test) + const corePoolReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + + // Donate enough to make balance significantly larger than tracked reserves + const donationAmount = parseUnits("1000000", 18); // Donate 1M USDT + await usdt.connect(whaleSigner).transfer(RISK_FUND_V2_PROXY, donationAmount); + + const newBalance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + console.log(` Donated ${ethers.utils.formatUnits(donationAmount, 18)} USDT`); + console.log(` New balance: ${ethers.utils.formatUnits(newBalance, 18)} USDT`); + console.log(` Tracked reserves (Core Pool): ${ethers.utils.formatUnits(corePoolReserve, 18)} USDT`); + + // Same sweep amount that caused revert pre-upgrade + const sweepAmount = corePoolReserve.add(parseUnits("100000", 18)); // Tracked + 100k + console.log(` Attempting to sweep: ${ethers.utils.formatUnits(sweepAmount, 18)} USDT`); + + const timelockBalanceBefore = await usdt.balanceOf(NORMAL_TIMELOCK); + + // With fixed code: uses amountDiff instead of amount, preventing underflow + await expect(riskFundV2.connect(timelockSigner).sweepToken(USDT, NORMAL_TIMELOCK, sweepAmount)).to.not.be + .reverted; + + // Verify tokens were actually swept + const timelockBalanceAfter = await usdt.balanceOf(NORMAL_TIMELOCK); + expect(timelockBalanceAfter.sub(timelockBalanceBefore)).to.equal(sweepAmount); + + console.log(" ✓ sweepToken succeeded after upgrade (fix confirmed)"); + console.log(` ✓ Timelock received ${ethers.utils.formatUnits(sweepAmount, 18)} USDT`); + } finally { + await snapshot.restore(); + } + }); + }); +}); diff --git a/simulations/vip-583/bsctestnet.ts b/simulations/vip-583/bsctestnet.ts new file mode 100644 index 000000000..bf5db55bf --- /dev/null +++ b/simulations/vip-583/bsctestnet.ts @@ -0,0 +1,232 @@ +import { + SnapshotRestorer, + impersonateAccount, + setBalance, + takeSnapshot, +} from "@nomicfoundation/hardhat-network-helpers"; +import { expect } from "chai"; +import { BigNumber, Contract, Signer } from "ethers"; +import { parseUnits } from "ethers/lib/utils"; +import { ethers } from "hardhat"; +import { forking, testVip } from "src/vip-framework"; + +import { + DEFAULT_PROXY_ADMIN, + RISK_FUND_V2_NEW_IMPLEMENTATION, + RISK_FUND_V2_PROXY, + vip583, +} from "../../vips/vip-583/bsctestnet"; +import PROXY_ADMIN_ABI from "./abi/ProxyAdmin.json"; +import RISK_FUND_V2_ABI from "./abi/RiskFundV2.json"; + +const USDT = "0x55d398326f99059fF775485246999027B3197955"; +const NORMAL_TIMELOCK = "0x939bD8d64c0A9583A7Dcea9933f7b21697ab6396"; + +// Core Pool Comptroller for testing poolAssetsFunds +const CORE_POOL_COMPTROLLER = "0xfD36E2c2a6789Db23113685031d7F16329158384"; + +// USDT whale for donation attack test (Binance Hot Wallet) +const USDT_WHALE = "0x8894E0a0c962CB723c1976a4421c95949bE2D4E3"; + +const ERC20_ABI = [ + "function balanceOf(address) view returns (uint256)", + "function transfer(address to, uint256 amount) returns (bool)", + "function approve(address spender, uint256 amount) returns (bool)", +]; + +forking(73352569, async () => { + let proxyAdmin: Contract; + let riskFundV2: Contract; + let usdt: Contract; + let oldImplementation: string; + let timelockSigner: Signer; + let whaleSigner: Signer; + + // State snapshots for comparison + let preUpgradeState: { + owner: string; + shortfall: string; + riskFundConverter: string; + convertibleBaseAsset: string; + corePoolUsdtReserve: BigNumber; + usdtBalance: BigNumber; + }; + + before(async () => { + proxyAdmin = await ethers.getContractAt(PROXY_ADMIN_ABI, DEFAULT_PROXY_ADMIN); + riskFundV2 = await ethers.getContractAt(RISK_FUND_V2_ABI, RISK_FUND_V2_PROXY); + usdt = await ethers.getContractAt(ERC20_ABI, USDT); + oldImplementation = await proxyAdmin.getProxyImplementation(RISK_FUND_V2_PROXY); + + // Impersonate timelock for later tests + await impersonateAccount(NORMAL_TIMELOCK); + await setBalance(NORMAL_TIMELOCK, parseUnits("100", 18)); + timelockSigner = await ethers.getSigner(NORMAL_TIMELOCK); + + // Impersonate USDT whale for donation attack tests + await impersonateAccount(USDT_WHALE); + await setBalance(USDT_WHALE, parseUnits("100", 18)); + whaleSigner = await ethers.getSigner(USDT_WHALE); + + // Snapshot pre-upgrade state + preUpgradeState = { + owner: await riskFundV2.owner(), + shortfall: await riskFundV2.shortfall(), + riskFundConverter: await riskFundV2.riskFundConverter(), + convertibleBaseAsset: await riskFundV2.convertibleBaseAsset(), + corePoolUsdtReserve: await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT), + usdtBalance: await usdt.balanceOf(RISK_FUND_V2_PROXY), + }; + }); + + describe("Pre-VIP state", async () => { + it("should have the old implementation", async () => { + expect(oldImplementation).to.equal("0x7Ef5ABbcC9A701e728BeB7Afd4fb5747fAB15A28"); + }); + + it("should have correct owner (Normal Timelock)", async () => { + expect(await riskFundV2.owner()).to.equal(NORMAL_TIMELOCK); + }); + + it("should have non-zero USDT balance", async () => { + const balance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + expect(balance).to.be.gt(0); + console.log(` Pre-upgrade USDT balance: ${ethers.utils.formatUnits(balance, 18)} USDT`); + }); + + it("should have tracked pool reserves", async () => { + const corePoolReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + console.log(` Core Pool USDT reserve: ${ethers.utils.formatUnits(corePoolReserve, 18)} USDT`); + }); + + it("BUG REPRODUCTION: donation attack causes sweepToken to revert (DoS)", async () => { + // This test demonstrates the bug being fixed: + // 1. Attacker donates tokens directly to the contract (untracked) + // 2. When governance tries to sweep amount > assetReserves, arithmetic underflow occurs + // + // Bug: preSweepToken used `amount` instead of `amountDiff` in share calculation. + // When amount > assetReserves: distributedShare (based on amount) > amountDiff, + // causing underflow in last pool: poolAmountShare = amountDiff - distributedShare + const snapshot: SnapshotRestorer = await takeSnapshot(); + + try { + const corePoolReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + + // Donate enough to make balance significantly larger than tracked reserves + const donationAmount = parseUnits("1000000", 18); // Donate 1M USDT + await usdt.connect(whaleSigner).transfer(RISK_FUND_V2_PROXY, donationAmount); + + const newBalance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + console.log(` Donated ${ethers.utils.formatUnits(donationAmount, 18)} USDT`); + console.log(` New balance: ${ethers.utils.formatUnits(newBalance, 18)} USDT`); + console.log(` Tracked reserves (Core Pool): ${ethers.utils.formatUnits(corePoolReserve, 18)} USDT`); + + // Sweep amount > assetReserves but < balance - this triggers the bug + // When amount > assetReserves: distributedShare calculated with `amount` exceeds amountDiff + const sweepAmount = corePoolReserve.add(parseUnits("100000", 18)); // Tracked + 100k + console.log(` Attempting to sweep: ${ethers.utils.formatUnits(sweepAmount, 18)} USDT`); + + // With buggy code: distributedShare (based on large `amount`) > amountDiff → underflow + await expect(riskFundV2.connect(timelockSigner).sweepToken(USDT, NORMAL_TIMELOCK, sweepAmount)).to.be.reverted; // Arithmetic underflow + + console.log(" ✓ sweepToken reverted as expected (bug confirmed)"); + } finally { + await snapshot.restore(); + } + }); + }); + + testVip("VIP-583 Upgrade RiskFundV2", await vip583()); + + describe("Post-VIP state", async () => { + it("should have the new implementation", async () => { + expect(await proxyAdmin.getProxyImplementation(RISK_FUND_V2_PROXY)).to.equal(RISK_FUND_V2_NEW_IMPLEMENTATION); + }); + + it("should preserve owner after upgrade", async () => { + expect(await riskFundV2.owner()).to.equal(preUpgradeState.owner); + }); + + it("should preserve shortfall contract after upgrade", async () => { + expect(await riskFundV2.shortfall()).to.equal(preUpgradeState.shortfall); + }); + + it("should preserve riskFundConverter after upgrade", async () => { + expect(await riskFundV2.riskFundConverter()).to.equal(preUpgradeState.riskFundConverter); + }); + + it("should preserve convertibleBaseAsset after upgrade", async () => { + expect(await riskFundV2.convertibleBaseAsset()).to.equal(preUpgradeState.convertibleBaseAsset); + }); + + it("should preserve poolAssetsFunds mapping after upgrade", async () => { + const postUpgradeReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + expect(postUpgradeReserve).to.equal(preUpgradeState.corePoolUsdtReserve); + }); + + it("should preserve token balances after upgrade", async () => { + const postUpgradeBalance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + expect(postUpgradeBalance).to.equal(preUpgradeState.usdtBalance); + }); + + it("should allow owner to call sweepToken (basic access check)", async () => { + // This test verifies the function is callable by owner + // We don't actually sweep funds, just verify the call doesn't revert on access + const balance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + + if (balance.gt(0)) { + // Sweep a small amount (1 USDT) to verify functionality + const sweepAmount = parseUnits("1", 18); + if (balance.gte(sweepAmount)) { + await expect(riskFundV2.connect(timelockSigner).sweepToken(USDT, NORMAL_TIMELOCK, sweepAmount)).to.not.be + .reverted; + } + } + }); + + it("should correctly report pool reserves via getPoolsBaseAssetReserves", async () => { + const reserves = await riskFundV2.getPoolsBaseAssetReserves(CORE_POOL_COMPTROLLER); + // Should return the same as poolAssetsFunds for base asset + console.log(` Core Pool base asset reserves: ${ethers.utils.formatUnits(reserves, 18)}`); + }); + + it("FIX VERIFICATION: donation attack no longer causes DoS after upgrade", async () => { + // This test verifies the fix works: + // Same scenario as pre-upgrade bug reproduction, but sweepToken should now succeed + const snapshot: SnapshotRestorer = await takeSnapshot(); + + try { + // Get current state to calculate proper test amounts (same as pre-upgrade test) + const corePoolReserve = await riskFundV2.poolAssetsFunds(CORE_POOL_COMPTROLLER, USDT); + + // Donate enough to make balance significantly larger than tracked reserves + const donationAmount = parseUnits("1000000", 18); // Donate 1M USDT + await usdt.connect(whaleSigner).transfer(RISK_FUND_V2_PROXY, donationAmount); + + const newBalance = await usdt.balanceOf(RISK_FUND_V2_PROXY); + console.log(` Donated ${ethers.utils.formatUnits(donationAmount, 18)} USDT`); + console.log(` New balance: ${ethers.utils.formatUnits(newBalance, 18)} USDT`); + console.log(` Tracked reserves (Core Pool): ${ethers.utils.formatUnits(corePoolReserve, 18)} USDT`); + + // Same sweep amount that caused revert pre-upgrade + const sweepAmount = corePoolReserve.add(parseUnits("100000", 18)); // Tracked + 100k + console.log(` Attempting to sweep: ${ethers.utils.formatUnits(sweepAmount, 18)} USDT`); + + const timelockBalanceBefore = await usdt.balanceOf(NORMAL_TIMELOCK); + + // With fixed code: uses amountDiff instead of amount, preventing underflow + await expect(riskFundV2.connect(timelockSigner).sweepToken(USDT, NORMAL_TIMELOCK, sweepAmount)).to.not.be + .reverted; + + // Verify tokens were actually swept + const timelockBalanceAfter = await usdt.balanceOf(NORMAL_TIMELOCK); + expect(timelockBalanceAfter.sub(timelockBalanceBefore)).to.equal(sweepAmount); + + console.log(" ✓ sweepToken succeeded after upgrade (fix confirmed)"); + console.log(` ✓ Timelock received ${ethers.utils.formatUnits(sweepAmount, 18)} USDT`); + } finally { + await snapshot.restore(); + } + }); + }); +}); diff --git a/vips/vip-583/bscmainnet.ts b/vips/vip-583/bscmainnet.ts new file mode 100644 index 000000000..0e8658f68 --- /dev/null +++ b/vips/vip-583/bscmainnet.ts @@ -0,0 +1,52 @@ +import { ProposalType } from "src/types"; +import { makeProposal } from "src/utils"; + +export const DEFAULT_PROXY_ADMIN = "0x6beb6D2695B67FEb73ad4f172E8E2975497187e4"; +export const RISK_FUND_V2_PROXY = "0xdF31a28D68A2AB381D42b380649Ead7ae2A76E42"; +export const RISK_FUND_V2_NEW_IMPLEMENTATION = "0x60e322C3418EcAEA5E6859551c299c968adc9816"; + +export const vip583 = () => { + const meta = { + version: "v2", + title: "Week VIP 2026 - Week 54 | Risk Fund Sweep Function Fix Proposal", + description: ` + ## 1. Context + +An edge case in the Risk Fund V2 contract related to the token sweeping mechanism was identified. Specifically, when the contract holds tokens that are not associated with any pool, the "preSweepToken" function may calculate pool-level token shares incorrectly. + +This can cause the sweep operation to revert, creating a denial-of-service scenario for governance-initiated token sweeps. While no user funds are directly at risk, this issue affects the reliability of protocol maintenance actions. + +## 2. Action Required + +A targeted fix is proposed to update the per-pool calculation logic so that it uses the actual amount required from pooled funds rather than the total requested sweep amount. This adjustment prevents arithmetic underflow when untracked tokens are present. + +The change is limited to a single line in the "RiskFundV2.sol" contract and does not modify external interfaces or user-facing behavior. If approved, this update would be executed via a VIP. + +## 3. Summary + +- An edge case in the Risk Fund sweep logic can cause governance sweep actions to revert +- The issue arises when untracked tokens exist in the contract balance +- The fix improves robustness without impacting users or fund safety +- Execution is subject to approval through a VIP + +We welcome community feedback on this proposal ahead of formal governance submission. + `, + forDescription: "Execute this proposal", + againstDescription: "Do not execute this proposal", + abstainDescription: "Indifferent to execution", + }; + + return makeProposal( + [ + { + target: DEFAULT_PROXY_ADMIN, + signature: "upgrade(address,address)", + params: [RISK_FUND_V2_PROXY, RISK_FUND_V2_NEW_IMPLEMENTATION], + }, + ], + meta, + ProposalType.REGULAR, + ); +}; + +export default vip583; diff --git a/vips/vip-583/bsctestnet.ts b/vips/vip-583/bsctestnet.ts new file mode 100644 index 000000000..96c711ec5 --- /dev/null +++ b/vips/vip-583/bsctestnet.ts @@ -0,0 +1,31 @@ +import { ProposalType } from "src/types"; +import { makeProposal } from "src/utils"; + +export const DEFAULT_PROXY_ADMIN = "0x7877ffd62649b6a1557b55d4c20fcbab17344c91"; +export const RISK_FUND_V2_PROXY = "0x487CeF72dacABD7E12e633bb3B63815a386f7012"; +export const RISK_FUND_V2_NEW_IMPLEMENTATION = "0x36b236c62AB32430016EC0EaE8e40ebe29d95869"; + +export const vip583 = () => { + const meta = { + version: "v2", + title: "VIP-583 Upgrade RiskFundV2 Implementation", + description: ``, + forDescription: "Execute this proposal", + againstDescription: "Do not execute this proposal", + abstainDescription: "Indifferent to execution", + }; + + return makeProposal( + [ + { + target: DEFAULT_PROXY_ADMIN, + signature: "upgrade(address,address)", + params: [RISK_FUND_V2_PROXY, RISK_FUND_V2_NEW_IMPLEMENTATION], + }, + ], + meta, + ProposalType.REGULAR, + ); +}; + +export default vip583;