Submitted by 0xRobocop, also found by stackachu, Toshii, Arz, DedOhWale, peanuts, Draiakoo, zhaojie, and ether_sky
Staking in SALTY pools happens automatically when adding liquidity. In order to track the accrued rewards, the code simulates the amount of virtual rewards that need to be added given the increase of shares and lend this amount to the user. So, when computing the real rewards for a given user, the code will compute its rewards based on the totalRewards of the given pool minus the virtual rewards. The following code computes the virtual rewards for a user:
https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L81
Basically, it aims to maintain the current ratio of totalRewards and existingTotalShares. The issue with this is that allows the first depositor to set the ratio too high by donating some SALT tokens to the contract. For example, consider the following values:
The returned value is in order of 39-40 digits. Which is beyond what 128 bits can represent:
https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L83-L84
This will broke the reward computations. For a more concrete example look the PoC.
The following coded PoC showcase an scenario where the first depositor set the rewards / shares ratio too high, causing the rewards system to get broken. Specifically, it shows how the sum of the claimable rewards for each user is greater than the SALT balance of the contract.
It should be pasted under Staking/tests/StakingRewards.t.sol.
Some options:
othernet-global (Salty.IO) confirmed and commented:
Picodes (Judge) commented:
Status: Mitigation confirmed. Full details in reports from t0x1c, 0xpiken, and zzebra83.
