Blur Exchange contest - s3cunda's results

Lines of code

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

Vulnerability details

Impact

_returnDust function havent check the status after using calls to send the remaining ETH back to caller, which will lead to some ETH locked in protocol when the msg.sender is a contract that do not have receive function.

furthermore, _returnDust function is not using remainingETH as the amount it send to caller, but using selfbalance as the amount, which means if someone's ETH locked in protocol as what I described above, other user can use this function to "steal" the locked ETH in the protocol

Proof of Concept

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

Tools Used

Recommended Mitigation Steps

add a status check in _ReturnDust function, record those failed ETH call allow admin or user to retrieve those locked ETH, using remainingETH as the amount not using selfbalance in order to avoid potential ETH stolen.