code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

Upgraded Q -> M from 270 [1657580410834] #366

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Judge has assessed an item in Issue #270 as Medium risk. The relevant finding follows:

6.L- Admin config ProtocolFee and gasFee missing max amount check which can be used to take fund from user

With PROTOCOL_FEE_BPS > 10000 (more than 100%), the exchange can steal user WETH who might approve max WETH allowance for InfinityExchange.sol To provide safety for user and in case of compromised both admin and MATCH_EXECUTOR bot server. Protocol should prevent potential further damage that might steal WETH from all user approve Exchange before.

function setProtocolFee(uint16 _protocolFeeBps) external onlyOwner {
    require(_protocolFeeBps <= 1000); //max 10%
    PROTOCOL_FEE_BPS = _protocolFeeBps; 
    emit NewProtocolFee(_protocolFeeBps);
  }

With maximum amount of WETH_TRANSFER_GAS_UNITS is 4_294_967_296 and default value is 50_000, basically exchange can take any amount of WETH from user if gas price set high enough.

  function updateWethTranferGas(uint32 _wethTransferGasUnits) external onlyOwner {
    require(_wethTransferGasUnits <= 200_000); // for deflation token or voting token transfer cannot really be more than 200000 gas per transaction
    WETH_TRANSFER_GAS_UNITS = _wethTransferGasUnits;
    emit NewWethTransferGasUnits(_wethTransferGasUnits);
  }
HardlyDifficult commented 2 years ago

Dupe https://github.com/code-423n4/2022-06-infinity-findings/issues/259