HardhatFoundry
30,000 USDC
View results
Submission Details
Severity: medium
Valid

Not compliant with ERC-7579 about `fallback` implemention

Summary

Not compliant with ERC-7579 about fallback implemention

Vulnerability Details

ERC-7579:

Fallback
Smart accounts MAY implement a fallback function that forwards the call to a fallback handler.
If the smart account has a fallback handler installed, it:
MUST implement authorization control
MUST use call to invoke the fallback handler
MUST utilize ERC-2771 to add the original msg.sender to the calldata sent to the fallback handler
MUST route to fallback handlers based on the function selector of the calldata

The fallback function is implemented as follows:

/// @dev Fallback function to manage incoming calls using designated handlers based on the call type.
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())
// The msg.sender address is shifted to the left by 12 bytes to remove the padding
// Then the address without padding is stored right after the calldata
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())
// The msg.sender address is shifted to the left by 12 bytes to remove the padding
// Then the address without padding is stored right after the calldata
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 difference with ERC-7579 are as follows:

  1. no access control

  2. allow staticcall, ERC-7579 only allows call

  3. don't implement ERC-2771 to get original msg.sender and msg.data

Impact

Not compliant with ERC-7579 about fallback implemention

Tools Used

manual

Recommendations

Change fallback function to be compliant with ERC-7579.

Updates

Lead Judging Commences

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

finding-fallback-missing-access-control-module-manager

There is indeed no access control within `fallback()` function which violates ERC7579 spec but the impact shown by all issues is insufficient. Need a better impact description/PoC that exceeds violation of ERC7579 to raise the severity of this issue. There will likely be no exploit for staticcall types, given there is not [state change/funds transfer allowed](https://www.rareskills.io/post/solidity-staticcall), so the possible vulnerability would be in the `CALLTYPE_SINGLE`. If no sufficient proof is provided to show a possible exploit, I will likely invalidate these issues.

Support

FAQs

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