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

Incorrect Offset Calculation in `parseEnableModeData`

rhaydden

Summary

The parseEnableModeData contains an error in the calculation and usage of offsets when parsing packed data. This error ccould cause incorrect slicing of the userOpSignature and may result in misinterpretation of user operation signatures.

Vulnerability Details

Lets look at parseEnableModeData function of the LocalCallDataParserLib library. The function is made to parse packed data, typically from a user operation's signature, to extract various components including module type, module initialization data, enable mode signature, and user operation signature.

But theres a mismatch between how an offset is calculated and how its subsequently used. Specifically:

The function uses inline assembly to efficiently parse the packed data:

function parseEnableModeData(bytes calldata packedData)
internal
pure
returns (
uint256 moduleType,
bytes calldata moduleInitData,
bytes calldata enableModeSignature,
bytes calldata userOpSignature
)
{
uint256 p;
assembly {
p := packedData.offset
// ... (other parsing logic)
p := sub(add(enableModeSignature.offset, enableModeSignature.length), packedData.offset)
}
userOpSignature = packedData[p:];
}

The p variable is initially set to packedData.offset and is then used to navigate through the packed data.

At the end of the assembly block, p is calculated as:

p := sub(add(enableModeSignature.offset, enableModeSignature.length), packedData.offset)

This calculation makes p a relative offset from packedData.offset.

However, p is then used as an absolute index when slicing packedData:

userOpSignature = packedData[p:];

This mismatch between relative offset calculation and absolute index usage leads to incorrect slicing of the userOpSignature.

Impact

Impact: HIGH

The userOpSignature may be sliced from an incorrect position in the packed data, leading to partial, incorrect, or completely missing signature data.

The ModuleManager.sol contract uses the parseEnableModeData function in its _enableMode method:

function _enableMode(address module, bytes calldata packedData) internal returns (bytes calldata userOpSignature) {
uint256 moduleType;
bytes calldata moduleInitData;
bytes calldata enableModeSignature;
(moduleType, moduleInitData, enableModeSignature, userOpSignature) = packedData.parseEnableModeData();
​
_checkEnableModeSignature(
_getEnableModeDataHash(module, moduleInitData),
enableModeSignature
);
_installModule(moduleType, module, moduleInitData);
}

The userOpSignature returned by parseEnableModeData is likely to be incorrect due to the offset calculation issue. This means that ModuleManager.sol is working with an incorrectly parsed signature.

Tools Used

Manual review

Recommendations

Modify the final calculation of p in the assembly block to make it an absolute index:

function parseEnableModeData(bytes calldata packedData)
internal
pure
returns (
uint256 moduleType,
bytes calldata moduleInitData,
bytes calldata enableModeSignature,
bytes calldata userOpSignature
)
{
uint256 p;
assembly {
p := packedData.offset
moduleType := calldataload(p)
moduleInitData.length := shr(224, calldataload(add(p, 0x20)))
moduleInitData.offset := add(p, 0x24)
p := add(moduleInitData.offset, moduleInitData.length)
​
enableModeSignature.length := shr(224, calldataload(p))
enableModeSignature.offset := add(p, 0x04)
- p := sub(add(enableModeSignature.offset, enableModeSignature.length), packedData.offset)
+ p := add(enableModeSignature.offset, enableModeSignature.length)
}
userOpSignature = packedData[p:];
}
Updates

Lead Judging Commences

0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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