Omniscia Boson Protocol Audit
EIP712Lib Manual Review Findings
EIP712Lib Manual Review Findings
EIP-01M: Limiting Signature Verification Process
| Type | Severity | Location |
|---|---|---|
| Logical Fault | ![]() | EIP712Lib.sol:L50-L52, L69-L71, L73-L85 |
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:
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 address50 * - Signer is a contract that does not implement ERC127151 * - Signer is a contract that implements ERC1271 but returns an unexpected value52 * - Signer is a contract that reverts when called with the signature53 * - Signer is an EOA but the signature is not a valid ECDSA signature54 * - Recovered signer does not match the user address55 *56 * @param _user - the message signer57 * @param _hashedMessage - hashed message58 * @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 ERC127166 bytes memory returnData; // Make this available for later if needed67 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 set77 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 signature91 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 unique99 // See https://github.com/OpenZeppelin/openzeppelin-contracts/blob/04695aecbd4d17dddfd55de766d10e3805d6f42f/contracts/cryptography/ECDSA.sol#63100 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 up112
113 /// @solidity memory-safe-assembly114 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.
