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

Incorrect Use of `msg.sender` in `isValidSignatureWithSender` Function Limits Signature Validation Flexibility in `K1Validator`

rhaydden

Summary

The isValidSignatureWithSender function ignores its first parameter (address) and instead uses msg.sender to determine the smart account owner.

Vulnerability Details

Look at this part of the code: https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/modules/validators/K1Validator.sol#L99-L109

function isValidSignatureWithSender(address, bytes32 hash, bytes calldata data) external view returns (bytes4) {
address owner = smartAccountOwners[msg.sender];
// Validate the signature using SignatureCheckerLib
if (SignatureCheckerLib.isValidSignatureNowCalldata(owner, hash, data)) {
return ERC1271_MAGICVALUE;
}
if (SignatureCheckerLib.isValidSignatureNowCalldata(owner, MessageHashUtils.toEthSignedMessageHash(hash), data)) {
return ERC1271_MAGICVALUE;
}
return ERC1271_INVALID;
}

The function ignores its first parameter (address) and uses msg.sender to determine the smart account owner. This can cause issues.
If the IValidator interface expects the first parameter to be used as the sender address, the current implementation doesnt meet this expectation.
Also, the function assumes that the smart account itself is always the caller, which might not be the case in more complex interactions, such as when called through a proxy or maybe another contract.

Impact

Inability to validate signatures for any account other than the one calling the function. Also, there'll be validation issues if the function is called with an address other than msg.sender. This is non-compliant with the expected behavior of the IValidator interface.

Tools Used

Manual review

Recommendations

isValidSignatureWithSender should use the provided address parameter instead of msg.sender.

function isValidSignatureWithSender(address, bytes32 hash, bytes calldata data) external view returns (bytes4) {
- address owner = smartAccountOwners[msg.sender];
+ address owner = smartAccountOwners[sender];
// Validate the signature using SignatureCheckerLib
if (SignatureCheckerLib.isValidSignatureNowCalldata(owner, hash, data)) {
return ERC1271_MAGICVALUE;
}
if (SignatureCheckerLib.isValidSignatureNowCalldata(owner, MessageHashUtils.toEthSignedMessageHash(hash), data)) {
return ERC1271_MAGICVALUE;
}
return ERC1271_INVALID;
}
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.