The addToPosition function in the MlumStaking contract has a vulnerability, and users may lose their voting eligibility

Summary

The addToPosition function in the MlumStaking contract has a vulnerability. The value of avgDuration may be equal to 0 due to floating-point rounding, which causes the lockDuration in the user's position to be equal to 0. When the user calls the Voter contract to vote, they cannot vote because lockDuration is equal to 0:

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L175-L177

Vulnerability Detail

Suppose Alice calls the createPosition function to create a position with an amount of 10e18 and a lockDuration of 90 days, which is 7776000. Please note the following code of the addToPosition function:

uint256 remainingLockTime = _remainingLockTime(position);
uint256 avgDuration = (remainingLockTime * position.amount + amountToAdd * position.initialLockDuration)
    / (position.amount + amountToAdd);

position.startLockTime = _currentBlockTimestamp();
position.lockDuration = avgDuration; // zero!

After 90 days, Alice's remainingLockTime is equal to 0. At this point, if Alice calls the addToPosition function and passes in a small value (such as 1) for the amountToAdd parameter, the formula will be calculated as follows:

uint256 avgDuration = ( 0 * 10e18 + 1 * 7776000) / (10e18 + 1) = 0

Since the numerator is smaller than the denominator, the value of avgDuration is equal to 0, resulting in the lockDuration of Alice's position also being equal to 0.

Alice will not be able to call the vote function of the Voter contract to vote.

The judgment statement of the vote function is extracted as follows:

if (_mlumStaking.getStakingPosition(tokenId).lockDuration < _periodDuration) {
    revert IVoter__InsufficientLockTime();
}

The key to this vulnerability is that floating-point rounding causes avgDuration to equal 0. If Alice does not strictly check the parameters when calling the addToPosition function, she may inadvertently try to vote for herself and fail.

At the same time, I noticed that the _requireOnlyOperatorOrOwnerOf function used by the addToPosition function can be bypassed by anyone, even if Alice herself does not call addToPosition again. Other malicious users may use this vulnerability to attack Alice.

Impact

The addToPosition function has a vulnerability that poses a potential risk of users being unable to vote

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L409-L414

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L175-L177

Tool used

Manual Review

Recommendation

Optimize the addToPosition function. If avgDuration=0, do not continue executing

require(avgDuration > 0, "0 Duration");

Duplicate of #138

sherlock-audit / 2024-06-magicsea-judging

neon2835 - The addToPosition function in the MlumStaking contract has a vulnerability, and users may lose their voting eligibility #675