Submitted by adriro, also found by Udsen, oualidpro, descharre, Rolezn, immeas, Breeje, jasonxiale and zaevlad.
The FeeWrapper contract wraps calls to an arbitrary target contract to add support for fees. The implementation executes a low level call to proxy the call to the target contract. This can be seen in the _rubicall and _rubicallPayable functions:
https://github.com/code-423n4/2023-04-rubicon/blob/main/contracts/utilities/FeeWrapper.sol#L60-L73
https://github.com/code-423n4/2023-04-rubicon/blob/main/contracts/utilities/FeeWrapper.sol#L76-L89
Low level calls behave differently than function calls in Solidity. Calls at the EVM level to accounts with no code are successful, this is the expected and normal behavior. It is Solidity that adds checks to prevent accidental calls to accounts with no code while compiling code for normal function calls.
This means that if the target account has no code, then the wrapped call using the FeeWrapper contract will succeed and the operation will be executed successfully. An accidental mistake may go unnoticed and may also cause unexpected loss of funds, as this call may include call value for transferring ETH.
In the following test, we use the FeeWrapper contract to execute a call to an account with no code. The transaction will succeed and the ETH will be transferred to the target account.
Note: the snippet shows only the relevant code for the test. Full test file can be found here.
While executing low level calls, the _rubicall and _rubicallPayable functions should check that either the account has code or the return data is greater than zero (which indicates the presence of an implementation). The OpenZeppelin contracts library provides utilities to execute low level calls in a safe way, including the recommended checks, present in the Address library, functionCall and functionCallWithValue.
daoio (Rubicon) confirmed
HickupHH3 (judge) commented:
