Omniscia Euler Finance Audit

EVCUtil Manual Review Findings

EVCUtil Manual Review Findings

EVC-01M: Inexistent Prevention of Modifier Misuse

TypeSeverityLocation
Logical FaultEVCUtil.sol:L29

Description:

The EVCUtil::callThroughEVC modifier co-exists with the EVCUitl::callThroughEVCPayable modifier with the latter equipped to handle native funds.

Impact:

Integrators of the EVCUtil contract may mistakenly specify the non-payable variant of the EVCUtil::callThorughEVC modifier for a payable function, an integration not prevented by the code itself.

Example:

src/utils/EVCUtil.sol
21/// @notice Ensures that the msg.sender is the EVC by using the EVC callback functionality if necessary.
22/// @dev Optional to use for functions requiring account and vault status checks to enforce predictable behavior.
23/// @dev If this modifier used in conjuction with any other modifier, it must appear as the first (outermost)
24/// modifier of the function.
25modifier callThroughEVC() virtual {
26 if (msg.sender == address(evc)) {
27 _;
28 } else {
29 bytes memory result = evc.call(address(this), msg.sender, 0, msg.data);
30
31 assembly {
32 return(add(32, result), mload(result))
33 }
34 }
35}
36
37/// @notice Ensures that the msg.sender is the EVC by using the EVC callback functionality if necessary.
38/// @dev Optional to use for functions requiring account and vault status checks to enforce predictable behavior.
39/// @dev If this modifier used in conjuction with any other modifier, it must appear as the first (outermost)
40/// modifier of the function.
41/// @dev This modifier is used for payable functions because it forwards the value to the EVC.
42modifier callThroughEVCPayable() virtual {
43 if (msg.sender == address(evc)) {
44 _;
45 } else {
46 bytes memory result = evc.call{value: msg.value}(address(this), msg.sender, msg.value, msg.data);
47
48 assembly {
49 return(add(32, result), mload(result))
50 }
51 }
52}

Recommendation:

We advise either a single implementation to be present, properly branching its code based on whether msg.value is non-zero, or the EVCUtil::callThroughEVC modifier to prevent a non-zero msg.value. Modifiers cannot control whether they are specified in a payable environment, and as such their misuse should be validated based on their intended operation.

Alleviation (577d1991de2aa6c1b0578e9e45d363e8d6799408):

The functionality of both the EVCUtil::callThroughEVC and EVCUtil::callThroughEVCPayable modifiers has been merged under a single implementation per our recommendation, utilizing a low-level assembly block to perform the interaction.

We have validated that the modifier is securely implemented, with a successful call properly accounting for the 32 byte data offset as well as 32 byte payload length in the return data.

As such, we consider this exhibit fully addressed with a uniform modifier for both payable and nonpayable interactions.