Missing default validator for on-chain signatures

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-party/blob/0ce3819de173f7688c9c834ce2cc758dd03c9bd2/contracts/proposals/ProposalExecutionEngine.sol#L225

Vulnerability details

Summary

For on-chain signatures, the validator routing design doesn't provide a default validator, requiring each individual hash to be defined using the SetSignatureValidatorProposal.

Impact

In the implementation of the EIP-1271 standard for the Party contract, the signature validation is done on a per hash basis. The SetSignatureValidatorProposal defines a mapping that associates hashes to their corresponding validator:

https://github.com/code-423n4/2023-10-party/blob/0ce3819de173f7688c9c834ce2cc758dd03c9bd2/contracts/proposals/SetSignatureValidatorProposal.sol#L8-L11

08:     struct SetSignatureValidatorProposalStorage {
09:         /// @notice Mapping from signature hash to signature validator for validating ERC1271 signatures.
10:         mapping(bytes32 => IERC1271) signatureValidators;
11:     }

This mapping is later queried by the ProposalExecutionEngine contract in order to determine which validator should be used when validating a signature request:

https://github.com/code-423n4/2023-10-party/blob/0ce3819de173f7688c9c834ce2cc758dd03c9bd2/contracts/proposals/ProposalExecutionEngine.sol#L225-L245

225:     function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4) {
226:         IERC1271 validator = getSignatureValidatorForHash(hash);
227:         if (address(validator) == address(1)) {
228:             // Signature set by party to be always valid
229:             return IERC1271.isValidSignature.selector;
230:         }
231:         if (address(validator) != address(0)) {
232:             return validator.isValidSignature(hash, signature);
233:         }
234:         if (tx.origin == address(0)) {
235:             validator = getSignatureValidatorForHash(0);
236:             if (address(validator) == address(0)) {
237:                 // Use global off-chain signature validator
238:                 validator = IERC1271(
239:                     _GLOBALS.getAddress(LibGlobals.GLOBAL_OFF_CHAIN_SIGNATURE_VALIDATOR)
240:                 );
241:             }
242:             return validator.isValidSignature(hash, signature);
243:         }
244:         return 0;
245:     }

As we can see in the previous snippet of code, for on-chain actions, validator routing is determined by the signature hash. Line 226 queries the mapping based on the given hash, and takes action based on the resulting address. As opposed to the off-chain case, all on-chain validation needs to be previously configured by executing a SetSignatureValidatorProposal for each individual hash that wants to be validated.

Recommendation

Introduce the capability to configure a default validator for on-chain actions. This feature could be implemented as an opt-in functionality, activated and customized by submitting a proposal, similar to the SetSignatureValidatorProposal case. By incorporating this optional feature, parties gain flexibility, eliminating the need to whitelist individual hashes for signature validation.

Assessed type

Other

[ydspa]

New feature rather than a bug

QA: NC

[c4-pre-sort]

ydspa marked the issue as insufficient quality report

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)