Biconomy: Nexus
Biconomy
HardhatFoundry
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

ModuleManager.sol#fallback() - The external calls will never work

EgisSecurity

Summary

The function is responsible for forwarding specific (enabled/installed) function selectors to their specific handler.

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())
}
}
}

Vulnerability Details


We'll focus on the assembly code, as that's the important part. Both call types have almost the same code, so we'll just use one of them for this issue.

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())
}

Let's imagine we want to call this function:

function handleCall(address caller) external pure returns (address) {
// Example function that just returns a fixed value
return caller;
}

The selector is 0x4a68832d. As msg.sender we'll use ab8483f64d9c6d1ecf9b849ae677dd3315835cb2

Let's go line by line and examine the assembly code:

  • calldatacopy(0, 0, calldatasize())-> Copies calldatasize() 4 bytes in our case bytes to memory at pointer 0

  • shl(96, caller() -> caller() is msg.sender in assembly and is always padded with 12 bytes in front: 000000000000000000000000ab8483f64d9c6d1ecf9b849ae677dd3315835cb2

  • This will shift the data 12 bytes to the left, which will look like this: 0xab8483f64d9c6d1ecf9b849ae677dd3315835cb2000000000000000000000000

  • mstore(calldatasize(), shl(96, caller())) -> This will store the shifted caller right after the copied calldata.

  • staticcall(gas(), handler, 0, add(calldatasize(), 20), 0, 0)-> This will make a call to handler with the data that starts from:

`inOffset = 0`
`inSize = 4 + 20 = 24 bytes`

  • So our data will look like this: 0x4a68832dab8483f64d9c6d1ecf9b849ae677dd3315835cb2

  • This will not work as it's not valid calldata. Valid calldata will be: 0x4a68832d000000000000000000000000ab8483f64d9c6d1ecf9b849ae677dd3315835cb2

  • You'll notice that it's 4 + 32 = 36 bytes long and the address isn't shifted.

This is how the calldata is supposed to look like, currently both CALLTYPE_STATIC and CALLTYPE_SINGLE won't work because of this, making fallback completely useless.

Impact

Functionality DoS

Tools Used

Manual Review

Recommendations

Apply the following changes:

​
mstore(calldatasize(), caller())
​
//@audit Do not copy only 20 bytes, copy all 32 bytes
if iszero(staticcall(gas(), handler, 0, add(calldatasize(), 0x20), 0, 0))
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!