KupiaSec - Incorrect calculation order in the `MlumStaking.addToPosition()` function when using `fee-on-transfer` tokens

[sherlock-admin4]

KupiaSec

Medium

Incorrect calculation order in the MlumStaking.addToPosition() function when using fee-on-transfer tokens

Summary

In the addToPosition() function, the transfer of tokens occurs after the calculation of the avgDuration. When using fee-on-transfer tokens, the actual amount received is smaller than the amount sent. This leads to an incorrect calculation of the avgDuration value.

Vulnerability Detail

When the stakedToken is a fee-on-transfer token, the actual received amount is smaller than the original amountToAdd. So, the calculation of avgDuration at L410 is done incorrectly, as it uses the original amountToAdd before the transfer. As a result, the lockDuration and the voting power are also incorrectly updated.

    function addToPosition(uint256 tokenId, uint256 amountToAdd) external override nonReentrant {
        ...

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

        ...

420     amountToAdd = _transferSupportingFeeOnTransfer(stakedToken, msg.sender, amountToAdd);

        ...
    }

Impact

Voting power is incorrectly updated when adding to existing positions.

Code Snippet

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

Tool used

Manual Review

Recommendation

The token transfer should be completed before the calculation of avgDuration is performed.

    function addToPosition(uint256 tokenId, uint256 amountToAdd) external override nonReentrant {
        ...

+       amountToAdd = _transferSupportingFeeOnTransfer(stakedToken, msg.sender, amountToAdd);
410     uint256 avgDuration = (remainingLockTime * position.amount + amountToAdd * position.initialLockDuration)
            / (position.amount + amountToAdd);

        ...

-       amountToAdd = _transferSupportingFeeOnTransfer(stakedToken, msg.sender, amountToAdd);

        ...
    }

Duplicate of #545