minhquanym - Anyone can steal fee in ExchangeProxy to do the swap

[sherlock-admin]

minhquanym

high

Anyone can steal fee in ExchangeProxy to do the swap

Summary

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

Vulnerability Detail

In ExchangeProxy.executeSwapDirect(...) function, user can pass in arbitrary _data. It's not validated to make sure this swap used exactly _amount tokenIn. In addition, fee of previous swaps is kept in ExchangeProxy. So attacker can steal these fee to do the swap.

Impact

Anyone can steal fee in ExchangeProxy

Code Snippet

Input token is transferred to ExchangeProxy contract before calling this function.

// ensure no state passed, no reentrancy, etc.
(bool success, ) = executorAddress.call{value: ethValue}(callData); // @audit can take fee of this contract to swap
require(success, "SWAP_CALL_FAILED");

Allowance to spender will be maxInt

// allow spender to transfer tokens from this contract
if (_tokenFrom != ETH_TOKEN_ADDRESS && spenderAddress != address(0)) {
    require(trustedRegistryContract.isWhitelisted(spenderAddress), "allowance to non-trusted");
    resetAllowanceIfNeeded(IERC20(_tokenFrom), spenderAddress, _amount);
}

Tool used

Manual Review

Recommendation

Consider transferring fee immediately after the swap. This contract is not supposed to hold any funds.

Duplicate of #112

[McMannaman]

Duplicate of
#37
#38
#52

I think that it's a low vulnerability (user funds are not affected by this and fees are harvested from time to time anyway in the normal flow of operation).
But, regardless -- this issue has a valid point.