1inch logo

1inch Limit Order Protocol & Settlement Security Review Report

April 2024

Summary

Total number of findings
1

Weaknesses

This section contains the list of discovered weaknesses.

AP1-1 | POTENTIAL UNDERFLOW IN FEETAKER CONTRACT

Severity:

Low

Path:

contracts/extensions/FeeTaker.sol:L26-L51

Description:

The FeeTaker.sol contract contains a potential risk of underflow in the calculation of fees if the fee percentage exceeds the limit of 1e7. In the FeeTaker::postInteraction() function, the fee is calculated by multiplying takingAmount with the fee percentage obtained from the extraData parameter. However, there is no explicit check to ensure that the fee percentage does not exceed the limit of 1e7.

unchecked {
    IERC20(order.takerAsset.get()).safeTransfer(receiver, takingAmount - fee);
}

Furthermore, the subtraction operation takingAmount - fee is currently marked with the unchecked keyword, which disables overflow and underflow checks. While this keyword is commonly used to optimize gas costs, it can pose a risk if the subtraction operation results in an underflow.

function postInteraction(
    IOrderMixin.Order calldata order,
    bytes calldata /* extension */,
    bytes32 /* orderHash */,
    address /* taker */,
    uint256 /* makingAmount */,
    uint256 takingAmount,
    uint256 /* remainingMakingAmount */,
    bytes calldata extraData
) external {
    uint256 fee = takingAmount * uint256(uint24(bytes3(extraData))) / _FEE_BASE;
    address feeRecipient = address(bytes20(extraData[3:23]));
 
    address receiver = order.maker.get();
    if (extraData.length > 23) {
        receiver = address(bytes20(extraData[23:43]));
    }
 
    if (fee > 0) {
        IERC20(order.takerAsset.get()).safeTransfer(feeRecipient, fee);
    }
 
    unchecked {
        IERC20(order.takerAsset.get()).safeTransfer(receiver, takingAmount - fee);
    }
}

Remediation:

Add a check to ensure that the fee percentage obtained from the extraData parameter does not exceed the limit of 1e7 or remove the unchecked keyword from the subtraction operation takingAmount - fee to enable automatic underflow checks by Solidity.

Table of contents