code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

Possible lock of staked xSALT if maxUnstakeWeek is reduced #850

Closed c4-bot-10 closed 9 months ago

c4-bot-10 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Staking.sol#L60-L79 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingConfig.sol#L51-L65

Vulnerability details

Impact

If a user is at week 52 and maxUnstakeWeeks is adjusted below 52, calling the calculateUnstake() function will always cause a revert due to the check require(numWeeks <= maxUnstakeWeeks, "Unstaking duration too long") in Staking.calculateUnstake, as maxUnstakeWeeks will be lower than 52, which is the number of weeks the user's token has been in the pool and is within the number of weeks required to unstake a token. This can lead to users being unable to unstake their tokens as intended, if maxUnstakeWeeks is not adjusted to allow the user unstake their tokens, the tokens become locked forever as it might stay beyond 108 weeks before the next adjustment.

Proof of Concept

By adjusting maxUnstakeWeeks below 52 as can be done in StakingConfig.changeMaxUnstakeWeeks

if (maxUnstakeWeeks > 20)
     maxUnstakeWeeks -= 8;

and attempting to call calculateUnstake() with numWeeks higher being 52 when calling Staking.unstake(), the function will revert due to the mentioned check in Staking.calculateUnstake():

require( numWeeks <= maxUnstakeWeeks, "Unstaking duration too long");

Tools Used

Recommended Mitigation Steps

Assessed type

Other

Picodes commented 9 months ago

What prevents the user from just calling unstake with the correct number of weeks?

c4-judge commented 9 months ago

Picodes marked the issue as unsatisfactory: Invalid