Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol?plain=1#L333

Vulnerability details

Impact

Not all IERC20 implementations revert() when there's a failure in transfer()/transferFrom(). The function signature has a boolean return value, and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment. Failure to properly handle transfer errors can be exploited by malicious actors to disrupt token transfers, potentially affecting the integrity and trust of the entire token ecosystem.

  function transfer(
        address recipient,
        uint256 amount
    ) public override(ERC20Minimal) returns (bool) {
        // make sure the caller does not have any open option positions
        // if they do: we don't want them sending panoptic pool shares to others
        // as this would reduce their amount of collateral against the opened positions

        if (s_panopticPool.numberOfPositions(msg.sender) != 0) revert Errors.PositionCountNotZero();

        return ERC20Minimal.transfer(recipient, amount);
    }

Proof of Concept

Scenario: Alice attempts to transfer 100 tokens to Bob. The ERC20Minimal.transfer call fails and returns false. Since the original function does not check the return value, it returns true, indicating a successful transfer. Bob never receives the tokens, and Alice's token balance remains unchanged, but Alice believes the transfer was successful. I have added a similar issue below as ref reference: https://consensys.io/diligence/audits/2020/09/aave-protocol-v2/#unhandled-return-values-of-transfer-and-transferfrom https://github.com/Uniswap/solidity-lib/blob/c01640b0f0f1d8a85cba8de378cc48469fcfd9a6/contracts/libraries/TransferHelper.sol#L33-L45

Tools Used

Manual review

Recommended Mitigation Steps

Ensure that the return value of the ERC20Minimal.transfer call is checked and handled appropriately. If the transfer fails, revert the transaction. Additionally, you could make use of OpenZeppelin's SafeERC20 wrapper functions which automatically handle return values and revert on failure.

https://github.com/Uniswap/solidity-lib/blob/c01640b0f0f1d8a85cba8de378cc48469fcfd9a6/contracts/libraries/TransferHelper.sol#L33-L45

Assessed type

ETH-Transfer

code-423n4 / 2024-06-panoptic-findings

Unhandled return value of transferFrom in contracts/CollateralTracker.sol #13