Links
https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/Nexus.sol#L109
https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/base/ModuleManager.sol#L169
https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/base/ModuleManager.sol#L394
https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/types/Constants.sol#L41
Summary
The declared module type hash doesn't match the hash calculated as per EIP 712, which can have an adverse effect when modules are being enabled.
Vulnerability Details
The MODULE_ENABLE_MODE_TYPE_HASH is declared as a hash of the module address and a the bytes32 initDataHash.
bytes32 constant MODULE_ENABLE_MODE_TYPE_HASH = keccak256("ModuleEnableMode(address module, bytes32 initDataHash)");
According to EIP-712 the Typehash is calculated as follows: typeHash = keccak256(encodeType(typeOf(s))).
Where encodeType is the type of a struct that is encoded as: name ‖ "(" ‖ member₁ ‖ "," ‖ member₂ ‖ "," ‖ … ‖ memberₙ ")".
The Typehash is calculated like this, using bytes initData instead of the expected bytes32 InitDataHash declared.
function _getEnableModeDataHash(address module, bytes calldata initData) internal view returns (bytes32 digest) {
digest = _hashTypedData(
keccak256(
abi.encode(
MODULE_ENABLE_MODE_TYPE_HASH,
module,
keccak256(initData)
)
)
);
}
As a result, the calculated hash will be incorrect, and will differ from the hash that would have been generated from an EIP712 generating tool.
Impact
There will be issues when the validateUserOp is called and the function attempts to enable mode
function validateUserOp(
PackedUserOperation calldata op,
bytes32 userOpHash,
uint256 missingAccountFunds
) external virtual payPrefund(missingAccountFunds) onlyEntryPoint returns (uint256 validationData) {
...
} else {
PackedUserOperation memory userOp = op;
userOp.signature = _enableMode(validator, op.signature);
validationData = IValidator(validator).validateUserOp(userOp, userOpHash);
}
}
This is because the _enableMode attempts to verify that the signature is correct.
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);
}
Tools Used
Manual Review
Recommendations
Rewrite the MODULE_ENABLE_MODE_TYPE_HASH
bytes32 constant MODULE_ENABLE_MODE_TYPE_HASH = keccak256("ModuleEnableMode(address module, bytes initData)");
