Blur Exchange contest - corerouter's results

Lines of code

https://github.com/code-423n4/2022-11-non-fungible/blob/main/contracts/Exchange.sol#L161 https://github.com/code-423n4/2022-11-non-fungible/blob/main/contracts/Exchange.sol#L209

Vulnerability details

Impact

The current implementation of _returnDust() does not check assembly call's return value. So even if the call does not succeed, the whole execute() transaction will succeed. In this way, a user's ETH may be stuck in the contract if _returnDust() fails to send back ETH and will be claimed by another user later on.

Proof of Concept

https://github.com/code-423n4/2022-11-non-fungible/blob/main/contracts/Exchange.sol#L161

https://github.com/code-423n4/2022-11-non-fungible/blob/main/contracts/Exchange.sol#L209

https://github.com/code-423n4/2022-11-non-fungible/blob/323b7cbf607425dd81da96c0777c8b12e800305d/contracts/Exchange.sol#L212-L227

Tools Used

Recommended Mitigation Steps

Modify current implementation of _returnDust() to check assembly call's return value and change the codes to as follows.

function _returnDust() private { bool callStatus = true;

    uint256 _remainingETH = remainingETH;
    assembly {
        if gt(_remainingETH, 0) {
            callStatus := call(
                gas(),
                caller(),
                selfbalance(),
                0,
                0,
                0,
                0
            )
        }
    }

   require(callStatus, "Return dust failed");
}