code-423n4 / 2024-03-saltyio-mitigation-findings

0 stars 0 forks source link

H-02 MitigationConfirmed #42

Open c4-bot-2 opened 4 months ago

c4-bot-2 commented 4 months ago

Lines of code

Vulnerability details

Lines of code

Vulnerability details

C4 Issue

https://github.com/code-423n4/2024-01-salty-findings/issues/614

Comments

The issue highlighted a unique edge case pertaining to the first depositor in a pool, how they can claim a full days worth of salt rewards. this was possible due to a combination of factors including:

  1. due to how rewards share calculation is done in _increaseUserShare function, the first depositor's rewards will be equal to the totalShares of the pool.
  2. Rewards to pools are distributed via the upkeep contract, which has a variable 'timeSinceLastUpkeep' determining the length of time passed since last upkeep. this is set on contract deployment.
  3. there is a delay between deployment of the upkeep contract and launch of exchange due the initial voting period that can last a few days.
  4. this delay means that when upkeep is called after launch, the timeSinceLastUpkeep will represent a dueration of a few days, which is then capped to a days worth of rewards .
  5. the first depositor can then call upkeep and claim a days worth of rewards.

Mitigation

https://github.com/othernet-global/salty-io/commit/4f0c9c6a6e3e4234135ab7119a0e380af3e9776c

As is evident in the code below, The final mitigation for this was to simply call upkeep right before the distribution of salt to all the relevant contract, which does two things:

  1. does not actually emit any rewards to pools because they haven't been distributed.
  2. resets the timeSinceLastUpkeep emission timer, so if upkeep is called by the user after distribution, only a small reward will be emitted to them.

https://github.com/othernet-global/salty-io/blob/4f0c9c6a6e3e4234135ab7119a0e380af3e9776c/src/launch/BootstrapBallot.sol#L76C1-L77C44

Tests

tests were added to check for this and are passing.

Conclusion

LGTM

c4-judge commented 4 months ago

Picodes marked the issue as satisfactory

c4-judge commented 4 months ago

Picodes marked the issue as confirmed for report