Skip to content

Commit

Permalink
fix: [ALC-LA2-3] remove SignatureType.CONTRACT case from MultiOwnerLi…
Browse files Browse the repository at this point in the history
…ghtAccount _validateSignature()
  • Loading branch information
jaypaik committed Apr 5, 2024
1 parent ac34254 commit a44a711
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
5 changes: 1 addition & 4 deletions src/MultiOwnerLightAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
/// @dev Implement template method of BaseAccount.
/// Uses a modified version of `SignatureChecker.isValidSignatureNow` in which the digest is wrapped with an
/// "Ethereum Signed Message" envelope for the EOA-owner case but not in the ERC-1271 contract-owner case.
/// The SignatureType.CONTRACT case is excluded here to prevent potential gas griefing attacks.
function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash)
internal
virtual
Expand All @@ -145,10 +146,6 @@ contract MultiOwnerLightAccount is BaseLightAccount, CustomSlotInitializable {
bytes32 signedHash = userOpHash.toEthSignedMessageHash();
bytes memory signature = userOp.signature[1:];
return _successToValidationData(_isValidEOAOwnerSignature(signedHash, signature));
} else if (signatureType == uint8(SignatureType.CONTRACT)) {
// Contract signature without address
bytes memory signature = userOp.signature[1:];
return _successToValidationData(_isValidContractOwnerSignatureNowLoop(userOpHash, signature));
} else if (signatureType == uint8(SignatureType.CONTRACT_WITH_ADDR)) {
// Contract signature with address
address contractOwner = address(bytes20(userOp.signature[1:21]));
Expand Down
36 changes: 22 additions & 14 deletions test/MultiOwnerLightAccount.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,6 @@ contract MultiOwnerLightAccountTest is Test {
assertTrue(lightSwitch.on());
}

function testExecuteCanBeCalledByEntryPointWithContractOwnerUnspecified() public {
_useContractOwner();
PackedUserOperation memory op = _getUnsignedOp(
abi.encodeCall(BaseLightAccount.execute, (address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ())))
);
op.signature =
abi.encodePacked(BaseLightAccount.SignatureType.CONTRACT, contractOwner.sign(entryPoint.getUserOpHash(op)));
PackedUserOperation[] memory ops = new PackedUserOperation[](1);
ops[0] = op;
entryPoint.handleOps(ops, BENEFICIARY);
assertTrue(lightSwitch.on());
}

function testExecuteCanBeCalledByEntryPointWithContractOwnerSpecified() public {
_useContractOwner();
PackedUserOperation memory op = _getUnsignedOp(
Expand Down Expand Up @@ -112,6 +99,27 @@ contract MultiOwnerLightAccountTest is Test {
entryPoint.handleOps(ops, BENEFICIARY);
}

function testRejectsUserOpWithContractOwnerUnspecified() public {
_useContractOwner();
PackedUserOperation memory op = _getUnsignedOp(
abi.encodeCall(BaseLightAccount.execute, (address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ())))
);
op.signature =
abi.encodePacked(BaseLightAccount.SignatureType.CONTRACT, contractOwner.sign(entryPoint.getUserOpHash(op)));
PackedUserOperation[] memory ops = new PackedUserOperation[](1);
ops[0] = op;

vm.expectRevert(
abi.encodeWithSelector(
IEntryPoint.FailedOpWithRevert.selector,
0,
"AA23 reverted",
abi.encodePacked(BaseLightAccount.InvalidSignatureType.selector)
)
);
entryPoint.handleOps(ops, BENEFICIARY);
}

function testRejectsUserOpWithInvalidContractOwnerSpecified() public {
PackedUserOperation memory op = _getUnsignedOp(
abi.encodeCall(BaseLightAccount.execute, (address(lightSwitch), 0, abi.encodeCall(LightSwitch.turnOn, ())))
Expand Down Expand Up @@ -715,7 +723,7 @@ contract MultiOwnerLightAccountTest is Test {
bytes32(uint256(uint160(0x0000000071727De22E5E9d8BAf0edAc6f37da032)))
)
),
0x38d4efa1969cecb0d91391970b4a410bfc1f26e6a41a29bebfda173a9a739ea0
0x2fff6cdec81f22cf680a4c06509a0e189084f530d5fdbfa5325940de2d89cbff
);
}

Expand Down

0 comments on commit a44a711

Please sign in to comment.