Locking.sol does not handle Fee-on-transfer tokens

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-10-loopfi/blob/main/src/Locking.sol#L59-L66

Vulnerability details

Impact

If the underlying token for Locking.sol is a Fee-on-transfer token, the protocol would not work as expected. Users may be unable to withdraw their tokens.

Bug Description

Note: This is a new issue that was introduced by the latest code diff (Doesn't exist in the 2024-07 Loopfi contest).

When users are calling deposit(), the deposits[msg.sender] is updated with the _amount that user passed in. However, _amount may not be the exact amount of tokens the protocol received, due to fee-on-transfer.

This means for each user, the accounting of his token is larger than expected. This would be an issue when user tries to withdraw tokens.

    function deposit(uint256 _amount) external {
        if (_amount == 0) revert AmountIsZero();

@>      deposits[msg.sender].amount += _amount;
        token.safeTransferFrom(msg.sender, address(this), _amount);

        emit Deposited(msg.sender, _amount);
    }

    function withdraw() external {
        uint256 _amount;
        if (cooldownPeriod > 0) {
            if (deposits[msg.sender].cooldownAmount == 0) revert NoTokensInCooldown();
            if (block.timestamp < deposits[msg.sender].cooldownStart + cooldownPeriod) revert CooldownPeriodNotPassed();
            _amount = deposits[msg.sender].cooldownAmount;
        } else {
            _amount = deposits[msg.sender].amount;
        }

        deposits[msg.sender].amount -= _amount;
        deposits[msg.sender].cooldownAmount = 0;
        deposits[msg.sender].cooldownStart = 0;
        token.safeTransfer(msg.sender, _amount);

        emit Withdrawn(msg.sender, _amount);
    }

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

    function deposit(uint256 _amount) external {
        if (_amount == 0) revert AmountIsZero();

-       deposits[msg.sender].amount += _amount;
+       uint256 balanceBefore = token.balanceOf(address(this));
        token.safeTransferFrom(msg.sender, address(this), _amount);
+       uint256 balanceAfter = token.balanceOf(address(this));
+       deposits[msg.sender].amount += balanceAfter - balanceBefore;

        emit Deposited(msg.sender, _amount);
    }

Assessed type

ERC20

[c4-judge]

koolexcrypto marked the issue as unsatisfactory:
Invalid