Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Handling of `msg.value` in `ModuleManager#fallback()`

sparrow

Summary

The ModuleManager#fallback() function is marked as payable, allowing it to accept Ether transfers. However, the current implementation does not handle msg.value correctly when making calls to designated handlers based on the call type. This can lead to core functionality issues when the function is expected to handle incoming Ether.

Vulnerability Details

The fallback function is designed to manage incoming calls using designated handlers based on the call type (either CALLTYPE_STATIC or CALLTYPE_SINGLE). The function does not take msg.value into account, causing the functionality to break when there is Ether attached to the call.

Here is the original fallback function:

fallback() external payable override(Receiver) receiverFallback {
FallbackHandler storage $fallbackHandler = _getAccountStorage().fallbacks[msg.sig];
address handler = $fallbackHandler.handler;
CallType calltype = $fallbackHandler.calltype;
require(handler != address(0), MissingFallbackHandler(msg.sig));
​
if (calltype == CALLTYPE_STATIC) {
assembly {
calldatacopy(0, 0, calldatasize())
mstore(calldatasize(), shl(96, caller()))
if iszero(staticcall(gas(), handler, 0, add(calldatasize(), 20), 0, 0)) {
returndatacopy(0, 0, returndatasize())
revert(0, returndatasize())
}
returndatacopy(0, 0, returndatasize())
return(0, returndatasize())
}
}
if (calltype == CALLTYPE_SINGLE) {
assembly {
calldatacopy(0, 0, calldatasize())
mstore(calldatasize(), shl(96, caller()))
if iszero(call(gas(), handler, 0, 0, add(calldatasize(), 20), 0, 0)) {
returndatacopy(0, 0, returndatasize())
revert(0, returndatasize())
}
returndatacopy(0, 0, returndatasize())
return(0, returndatasize())
}
}
}

The issue lies in the omission of msg.value when making the call. For CALLTYPE_SINGLE, the value should be forwarded, whereas CALLTYPE_STATIC should not handle Ether transfers.

Impact

The core functionality of the fallback mechanism is broken when msg.value is expected to be transferred with the call. This can lead to significant issues, including:

  • Failure to execute intended actions if Ether is required by the handler.

  • Potential loss of Ether if the handler does not properly handle the transfer.

  • Inconsistent behavior of the fallback function, leading to security vulnerabilities and system instability.

Tools Used

Manual code review

Recommendations

To resolve the issue, update the fallback function to correctly handle msg.value for CALLTYPE_SINGLE

Updates

Lead Judging Commences

0xnevi Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-cannot-msg.value-not-forwarded

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.