sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

GoSlang - _setMintUnlockTime is unfair for users using the protocol #60

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

GoSlang

medium

_setMintUnlockTime is unfair for users using the protocol

Summary

Users are awarded points for opening leveraged positions, these points are placed under a timelock and this timelock is updated as follows: https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/PointsModule.sol#L142-L155

Vulnerability Detail

let's consider the first scenario from the code snippet below where we have not unlocked our tokens yet there are two problems here, the protocol does not take into account how much time the user has left and how much the user is minting consider the following:

UserA creates a leveraged position for 100 UserA waits until the very last second of the unlockTaxVest and adjust the additionalSize by one UserA will have to wait for the whole unlockTaxVest again to get the large amount of tokens that were already about to be released without making a significant change in the position.

This also has the flaw of being bad for people that wanted to add a little more to their position since that would double their time vs if they would have done it all in the first transaction.

The second path is also not flawless since if a user has already unlocked their tokens they now don't want to create any more position in the protocol since that would lock their tokens again.

Impact

Users are incentivized not to use the protocols leverage trading more than once since they get locked out of their points.

Code Snippet

    function _setMintUnlockTime(address account, uint256 mintAmount) internal returns (uint256 newUnlockTime) {
        uint256 lockedAmount = _lockedAmount[account];
        uint256 unlockTimeBefore = unlockTime[account];

        if (unlockTimeBefore <= block.timestamp) {
            newUnlockTime = block.timestamp + unlockTaxVest;
        } else {
            uint256 newUnlockTimeAmount = (block.timestamp + unlockTaxVest) * mintAmount;
            uint256 oldUnlockTimeAmount = unlockTimeBefore * (lockedAmount - mintAmount);
            newUnlockTime = (newUnlockTimeAmount + oldUnlockTimeAmount) / lockedAmount;
        }

        unlockTime[account] = newUnlockTime;
    }

Tool used

Manual Review

Recommendation

update the unlockTimeBefore <= block.timestamp to factor in how much the user is minting and how long it is left. for the second path we can track the unlocked balance separately from the locked balance

Duplicate of #200

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid