Infinity NFT Marketplace contest - byterocket's results

Lines of code

https://github.com/infinitydotxyz/exchange-contracts-v2/blob/main/contracts/core/InfinityExchange.sol#L1230 https://github.com/infinitydotxyz/exchange-contracts-v2/blob/main/contracts/core/InfinityExchange.sol#L326 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L346 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L55

Vulnerability details

Description

The functions rescueETH(address) in the InifinityExchange and InfinityStaker contracts are intended to send all ETH held in the contract to the address given as function argument.

However, the function implementations differ from the specification in that they only forward the ETH send in the call's msg.value.

Impact

The impact is HIGH because ETH held in the contracts can not be withdrawn/rescued, leading to a loss of all ETH held in the contract.

As the contracts implements a fallback and a receive function, the contracts are definitly able to receive ETH.

Recommendation

Refactor the rescueETH functions to something like:

function rescueETH(address to) external onlyOwner {
    uint balance = address(this).balance;
    (bool sent, ) = destination.call{value: balance}('');
    require(sent, 'failed');
}

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L326 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L362

Vulnerability details

Description

Users executing a maker order to buy a NFT can loose ETH when using the takeOrders() or takeMultipleOneOrders() functions.

This is due to a require statement checking that msg.value >= totalPrice.

This leads to the possiblity for a user to accidently overpay for a NFT, without possibility to receive the overpayed ETH amount back.

Impact

The impact is MEDIUM as this issue can lead to a loss of funds for users.

Recommendation

Refactor the require statements to check that the exact amount of ETH for the trade was send, i.e.

require(msg.value == totalPrice, "invalid total price");