Omniscia Euler Finance Audit
EVCUtil Manual Review Findings
EVCUtil Manual Review Findings
EVC-01M: Inexistent Prevention of Modifier Misuse
Type | Severity | Location |
---|---|---|
Logical Fault | EVCUtil.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:
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.