sherlock-audit / 2023-10-looksrare-judging

6 stars 6 forks source link

BoRonGod - function `heal` is broken due to invalid approval #79

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

BoRonGod

high

function heal is broken due to invalid approval

Summary

Users are unable to use the heal function in InfiltrationPeripheral.sol normally.

Vulnerability Detail

In function heal, user's $ETH is exchanged for $LOOKS in uniswap v3, and the codes below would be executed:

IERC20(LOOKS).approve(address(TRANSFER_MANAGER), costToHealInLOOKS);
INFILTRATION.heal(agentIds);

It is worth noting that, instead of approve for Infiltration, it approves for TRANSFER_MANAGER. However, when it tries to call INFILTRATION.heal(agentIds), the logic used to transfer tokens is

TRANSFER_MANAGER.transferERC20(LOOKS, msg.sender, address(this), cost);

In TRANSFER_MANAGER.transferERC20, _executeERC20TransferFrom in LowLevelERC20Transfer.sol is called:

// Excerpt from TransferManager.sol
function transferERC20(
    address tokenAddress,
    address from,
    address to,
    uint256 amount
) external {
    _isOperatorValidForTransfer(from, msg.sender);

    if (amount == 0) {
        revert AmountInvalid();
    }

    _executeERC20TransferFrom(tokenAddress, from, to, amount);
}

// Excerpt from LowLevelERC20Transfer.sol
function _executeERC20TransferFrom(address currency, address from, address to, uint256 amount) internal {
    if (currency.code.length == 0) {
        revert NotAContract();
    }

    (bool status, bytes memory data) = currency.call(abi.encodeCall(IERC20.transferFrom, (from, to, amount)));

    if (!status) {
        revert ERC20TransferFromFail();
    }

    if (data.length > 0) {
        if (!abi.decode(data, (bool))) {
            revert ERC20TransferFromFail();
        }
    }
}

As shown in the code above, the approval of $LOOKS should be given to Infiltration contract instead of transer manager.

Impact

function heal is broken.

Code Snippet

https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/InfiltrationPeriphery.sol#L60 https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L913 https://github.com/LooksRare/contracts-transfer-manager/blob/29b789149fb798c10445c0bed52e2ad3aa4d3fb7/contracts/TransferManager.sol#L54 https://github.com/LooksRare/contracts-libs/blob/master/contracts/lowLevelCallers/LowLevelERC20Transfer.sol#L24C33-L24C33

Tool used

Manual Review

Recommendation

Use IERC20(LOOKS).approve(address(Infiltration), costToHealInLOOKS);instead of IERC20(LOOKS).approve(address(TRANSFER_MANAGER), costToHealInLOOKS);

nevillehuang commented 1 year ago

approval required to TRANSFER_MANAGER here since it is the caller in Infiltration.heal()