Blur Exchange contest - carlitox477's results

Lines of code

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

Vulnerability details

Impact

After sending ETh through call, the callStatus is not verified. If call does not succeed and there is gas remain, the operation won't revert, and the ETH will stay in the contract locked

Proof of Concept

Alice deploy a smart contract called RussianRullet that in certain conditions, when it receives ETH through receive function, can revert.

This contract also has a function called tryLuck which just calls Exchange.execute{value:msg.value([])} function

1. Alice calls tryLuck with 1 ETH
2. Exchange.execute{value:msg.value([])} is executed
3. _returnDust is executed, and ETH is send back to RussianRullet smart contract
4. The call to RussianRullet smart contract for some reason reverts
5. _returnDust end its execution

The ETH send by Alice is now stuck in the contract.

Tools Used

Manual review

Recommended Mitigation Steps

Check callStatus value, and in case it is false, revert the operation.

Pool#withdraw can reduce one state variable access

_balances[msg.sender] can reduce one state variable access. Replace these lines for:

uitn256 _balance = _balances[msg.sender]
require(_balance >= amount);
_balance -= amount;
 _balances[msg.sender] = _balance

Pool#transferFrom can use a custom error to reduce gas consumption

This line can be replaced by something like revert(ERROR_UNAUTORIZED_TRANSFER());

Note: Notice that this error was not caught by c4audit

Pool#_transfer can reduce one state variable access

_balances[from] is accessed three times. This can be reduced to just 2 by replacing these lines for:

uint256 fromBalance = _balances[from];
require(fromBalance >= amount);
require(to != address(0));
fromBalance -= amount;
_balances[from] = fromBalance
_balances[to] += amount;

Exchange#_executeFundsTransfer can reduce one state variable access

remainingETH is accessed 3 times. One access can be avoided by replacing these lines for

uint256 _remainingETH = remainingETH;
require(_remainingETH >= price);
_remainingETH -= price;
remainingETH = _remainingETH;

Exchange#setBlockRange can reduce one state variable access

blockRange is accessed 2 times. This can be reduced to just 1 by replacing this lines for emit NewBlockRange(_blockRange);

Exchange#setOracle can reduce one state variable access

oracle is accessed 2 times. This can be reduced to just 1 by replacing this line for emit NewOracle(_oracle);

Exchange#setPolicyManager can reduce one state variable access

policyManager is accessed 2 times. This can be reduced to just 1 by replacing this line for emit NewPolicyManager(_policyManager);

Exchange#setExecutionDelegate can reduce one state variable access

executionDelegate is accessed 2 times. This can be reduced to just 1 by replacing this line for emit NewExecutionDelegate(_executionDelegate);

Exchange#incrementNonce can reduce one state variable access

nonces[msg.sender] is accessed 3 times. This can be reduced to just 1 by replacing these lines for:

nonce = nonces[msg.sender] + 1
nonces[msg.sender] = nonce;
emit NonceIncremented(msg.sender, nonce);