Offchain signature can be improved

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/signature-validators/OffChainSignatureValidator.sol#L28

Vulnerability details

Impact

Offchain signature can be improved

Proof of Concept

This issue intends to combine a few issues,

1. signature never expires
2. signature schema is too simple,
   • no chain id for cross-chain replay protection, the protocol does intend to deploy on both ethereum and base network
   • no nonce increment for signature replay protection, meaning the same signature can be used to bypass the signature validation multiple times as long as the owner is a party member
   • only the message length is in the signature schema, but the same message that have message length can be decoded in different way

relevant line of code

// Recreate the message pre-hash from the raw data
bytes memory encodedPacket = abi.encodePacked(
    "\x19Ethereum Signed Message:\n",
    Strings.toString(message.length),
    message
);
if (keccak256(encodedPacket) != hash) {
    revert MessageHashMismatch();
}

3. because signature never expires and no replay protection, in the case when the totalWeight is changed and reduced by govnerance

// Either threshold is 0 or signer votes above threshold
if (
    thresholdBps == 0 ||
    (signerVotingPowerBps > totalVotingPower &&
        signerVotingPowerBps / totalVotingPower >= thresholdBps)
) {
    return IERC1271.isValidSignature.selector;
}

an outdated signature can be used to bypass the validation without generating new signature

suppose

signerVotingPowerBps is 1000

totalVotingPower is 500

thresholdBps is 3

1000 / 500 >= 3 is false

but in totalVotingPower changed and reduced to 200

1000 / 200 >= 3 is true, and signature validation is passed

Tools Used

Manual Review

Recommended Mitigation Steps

add nonce to avoid signature reuse

add signature expiration to make sure signature have expiration date

Assessed type

Invalid Validation

[c4-pre-sort]

ydspa marked the issue as sufficient quality report

[c4-pre-sort]

ydspa marked the issue as primary issue

[arr00]

The intended usecase for the off-chain validator is sign in with ethereum which requires an address, chainId, and nonce to be part of the message. https://docs.login.xyz/general-information/siwe-overview/eip-4361.

When using SIWE for a website off-chain, the website gives a message and retains the hash of that message for validation. We are unable to change the message to add other arbitrary data to it as that would change the hash and invalidate the signature.

[c4-sponsor]

arr00 (sponsor) disputed

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid