sherlock-audit / 2022-10-mover-judging

1 stars 0 forks source link

supernova - Funds can get locked during ExecuteSwap #73

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

supernova

high

Funds can get locked during ExecuteSwap

Summary

The executeSwap function in exchangeProxy , which helps swap for the user , has a missing check , which can potentially lock user's funds.

Vulnerability Detail

The executeSwap function allows user to swap ETH to ERC20, ER20 to ETH . Let's take a specific case : Alice calls executeSwap with the following params: _tokenFrom = Any ERC20 address __tokenTo = ERC20
_amount = any amount msg.value = 1 Ether

In this case ,the contract transfers itself the ERC20 tokens using safeTransferFrom , but there is no check , which prevents the user to send any value if the _tokenFrom is not 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE.

Impact

The user's funds will be permanently locked in this case.

Code Snippet

https://github.com/sherlock-audit/2022-10-mover/blob/main/cardtopup_contract/contracts/ExchangeProxy.sol#L91

function executeSwap(
        address _tokenFrom,
        address _tokenTo,
        uint256 _amount,
        bytes memory _data
    ) public payable override returns (uint256) {
        // native token doesn't need to be transferred explicitly, it's in tx.value
        if (_tokenFrom != ETH_TOKEN_ADDRESS) {
            IERC20(_tokenFrom).safeTransferFrom(msg.sender, address(this), _amount);
        }
        // after token is transferred to this contract, call actual swap
        return executeSwapDirect(msg.sender, _tokenFrom, _tokenTo, _amount, 0, _data);
    }

Tool used

VsCode , Foundry Manual Review

Recommendation

I recommend adding the following check in the executeSwap function


if(_tokenFrom != ETH_TOKEN_ADDRESS){
require(msg.value ==0 );
}