Inconsistent Token Transfer Handling Leading to Potential Fund Loss

Summary

The FlapperUniV2 uses the GemLike.transfer method to transfer tokens without verifying the return value. This can lead to undetected transfer failures, causing discrepancies in token balances and potential fund loss.

Vulnerability Detail

The FlapperUniV2 interacts with ERC-20 tokens using the GemLike.transfer method. According to the ERC-20 standard, the transfer function should return a boolean value indicating the success of the operation. However, some tokens do not adhere to this standard and may not return any value or may return false upon failure. The current implementation does not check the return value of the transfer function, assuming it always succeeds. This can lead to scenarios where token transfers fail silently, causing the contract to behave incorrectly. GemLike(dai).transfer(address(pair), _sell); GemLike(dai).transfer(address(pair), lot - _sell); GemLike(gem).transfer(address(pair), _buy);

Impact

Silent Transfer Failures
Potential Fund Loss
Operational Disruption
Security Risks

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/FlapperUniV2.sol#L141-L164 https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/FlapperUniV2.sol#L152 https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/FlapperUniV2.sol#L158 https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/FlapperUniV2.sol#L159

Tool used

Manual Review

Recommendation

Utilize a library like OpenZeppelin's SafeERC20 to handle token transfers safely. This library includes checks for the return value and reverts the transaction if the transfer fails.
```
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
```

contract FlapperUniV2 { using SafeERC20 for IERC20;

// Replace GemLike with IERC20
IERC20 public dai;
IERC20 public gem;

// Update transfer calls to use SafeERC20
function exec(uint256 lot) external auth {
    // ... other logic ...

    // Safe transfer
    dai.safeTransfer(address(pair), _sell);
    dai.safeTransfer(address(pair), lot - _sell);
    gem.safeTransfer(address(pair), _buy);

    // ... other logic ...
}

}

- If not using a library, manually check the return value of the transfer function to ensure it succeeded.
```solidity
function safeTransfer(GemLike token, address to, uint256 amount) internal {
    bool success = token.transfer(to, amount);
    require(success, "Token transfer failed");
}

function exec(uint256 lot) external auth {
    // ... other logic ...

    // Safe transfer
    safeTransfer(GemLike(dai), address(pair), _sell);
    safeTransfer(GemLike(dai), address(pair), lot - _sell);
    safeTransfer(GemLike(gem), address(pair), _buy);

    // ... other logic ...
}

sherlock-audit / 2024-06-makerdao-endgame-judging

JuggerNaut63 - Inconsistent Token Transfer Handling Leading to Potential Fund Loss #67