Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L374-L380

Vulnerability details

Impact

The _lock function in the LockManager contract uses the transferFrom method to transfer tokens. The contract claims to support fee-on-transfer tokens, which deduct a fee from the transferred amount. However, the function does not account for the actual amount received, which leads to discrepancies between the intended and actual locked quantities. This can result in incorrect token balances, miscalculations, and potential financial loss for the protocol.

Proof of Concept

In the LockManager contract, the _lock function includes the following code:

if (_tokenContract != address(0)) {
    IERC20 token = IERC20(_tokenContract);
    token.transferFrom(_tokenOwner, address(this), _quantity);
}

lockedToken.remainder = remainder;
lockedToken.quantity += _quantity;

When a fee-on-transfer token is used, the transferFrom call deducts a fee, meaning the actual amount received by the contract is less than _quantity. For instance, if the user specifies 100 tokens and a 2% fee is applied, only 98 tokens are received. The function, however, assumes 100 tokens are received, leading to incorrect balances and potential financial discrepancies.

Tools Used

Manual code review

Recommended Mitigation Steps

To accurately reflect the received token amount, calculate the difference in token balance before and after the transferFrom call and update the lockedToken.quantity accordingly. Here’s how you can refactor the _lock function:

Import the SafeERC20 library:

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

Use safeTransferFrom and calculate the received amount:

if (_tokenContract != address(0)) {
   IERC20 token = IERC20(_tokenContract);
   uint256 balanceBefore = token.balanceOf(address(this));
   SafeERC20.safeTransferFrom(token, _tokenOwner, address(this), _quantity);
   uint256 balanceAfter = token.balanceOf(address(this));
   uint256 receivedAmount = balanceAfter - balanceBefore;

   lockedToken.quantity += receivedAmount;
}

This approach ensures that the actual amount received is accurately reflected in the lockedToken.quantity, thereby preventing miscalculations and financial discrepancies.

By implementing these changes, the contract will handle fee-on-transfer tokens correctly, maintaining accurate token balances and preventing potential financial losses for the protocol.

Assessed type

Token-Transfer

code-423n4 / 2024-05-munchables-findings

Fee-On-Transfer Tokens Are Not Supported #516