Omniscia Boson Protocol Audit

EIP712Lib Manual Review Findings

EIP712Lib Manual Review Findings

EIP-01M: Limiting Signature Verification Process

Description:

The EIP712Lib::verify function is meant to verify that a particular hashed message has been authorized either through EIP-1271 or signature validation.

The current approach's error handling seems inconsistent, as the function implies a revert will occur if the _user is a contract that does not implement EIP-1271 which is not the case.

Additionally, the implementation does not seem to account for smart accounts (EIP-7702) and will fatally revert in case EIP-1271 is not adhered to strictly even though signature validation might ultimately succeed.

Impact:

The documentation presently implies a revert case that is unfulfilled, and will revert in certain edge cases that would otherwise succeed in signature validation of the previous implementation.

Example:

contracts/protocol/libs/EIP712Lib.sol
44/**
45 * @notice Verifies that the signer really signed the message.
46 * It works for both ECDSA signatures and ERC1271 signatures.
47 *
48 * Reverts if:
49 * - Signer is the zero address
50 * - Signer is a contract that does not implement ERC1271
51 * - Signer is a contract that implements ERC1271 but returns an unexpected value
52 * - Signer is a contract that reverts when called with the signature
53 * - Signer is an EOA but the signature is not a valid ECDSA signature
54 * - Recovered signer does not match the user address
55 *
56 * @param _user - the message signer
57 * @param _hashedMessage - hashed message
58 * @param _signature - signature. If the signer is EOA, it must be ECDSA signature in the format of (r,s,v) struct, otherwise, it must be a valid ERC1271 signature.
59 */
60function verify(address _user, bytes32 _hashedMessage, bytes calldata _signature) internal {
61 if (_user == address(0)) revert BosonErrors.InvalidAddress();
62
63 bytes32 typedMessageHash = toTypedMessageHash(_hashedMessage);
64
65 // Check if user is a contract implementing ERC1271
66 bytes memory returnData; // Make this available for later if needed
67 if (_user.code.length > 0) {
68 bool success;
69 (success, returnData) = _user.staticcall(
70 abi.encodeCall(IERC1271.isValidSignature, (typedMessageHash, _signature))
71 );
72 if (success) {
73 if (returnData.length != SLOT_SIZE) {
74 revert BosonErrors.UnexpectedDataReturned(returnData);
75 } else {
76 // Make sure that the lowest 224 bits (28 bytes) are not set
77 if (uint256(bytes32(returnData)) & type(uint224).max != 0) {
78 revert BosonErrors.UnexpectedDataReturned(returnData);
79 }
80
81 if (abi.decode(returnData, (bytes4)) != IERC1271.isValidSignature.selector)
82 revert BosonErrors.SignatureValidationFailed();
83
84 return;
85 }
86 }
87 }
88
89 address signer;
90 // If the user is not a contract or the call to ERC1271 failed, we assume it's an ECDSA signature
91 if (_signature.length == 65) {
92 ECDSASignature memory ecdsaSig = ECDSASignature({
93 r: bytes32(_signature[0:32]),
94 s: bytes32(_signature[32:64]),
95 v: uint8(_signature[64])
96 });
97
98 // Ensure signature is unique
99 // See https://github.com/OpenZeppelin/openzeppelin-contracts/blob/04695aecbd4d17dddfd55de766d10e3805d6f42f/contracts/cryptography/ECDSA.sol#63
100 if (
101 uint256(ecdsaSig.s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0 ||
102 (ecdsaSig.v != 27 && ecdsaSig.v != 28)
103 ) revert BosonErrors.InvalidSignature();
104
105 signer = ecrecover(typedMessageHash, ecdsaSig.v, ecdsaSig.r, ecdsaSig.s);
106 if (signer == address(0)) revert BosonErrors.InvalidSignature();
107 }
108
109 if (signer != _user) {
110 if (returnData.length > 0) {
111 // In case 1271 verification failed with a revert reason, bubble it up
112
113 /// @solidity memory-safe-assembly
114 assembly {
115 revert(add(SLOT_SIZE, returnData), mload(returnData))
116 }
117 }
118
119 revert BosonErrors.SignatureValidationFailed();
120 }
121}

Recommendation:

We advise the code to become EIP-7702 conscious and to not revert fatally in its EIP-1271 integration, updating its documented error cases in the process.

Alleviation (efd5d1a8f23c3bca7c25273ea4c912a367250119):

The code was updated to properly proceed with ECDSA validation regardless of the failure that was identified during the EIP-1271 recovery flow, bubbling up the appropriate error should both validations fail.

As such, we consider this exhibit fully alleviated.