Rigor Protocol contest - Soosh's results

Low Risk Findings

Signatures can be reused on different chains

• EIP712 domain is not used for the MetaTxs. https://eips.ethereum.org/EIPS/eip-712

• This allows an attacker to replay a signed transaction on another app, version or chain.

• The vulnerability lies in no domain seperator included in the hashes being signed in checkSignatureValidity()

• In Rigor, there is a lot of variability in the hash being signed. For example, in addMember(), to successfully do a replay attack, the _communityID and _community.owner would have to match and you will be limited to adding only the same _newMemberAddr.

• Unlikely isn't impossible and there is still a risk to the protocol's integrity.

Recommendations

• A domain seperator should be included as part of the hash being signed. This prevents an attacker from replaying signed MetaTxs on other domains.
• Taken from OZ, the below shows the digest being signed, which includes the domain seperator into the final hash to be signed

bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(
keccak256("Mail(address to,string contents)"),
mailTo,
keccak256(bytes(mailContents))
)));
address signer = ECDSA.recover(digest, signature);

• See (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/draft-EIP712.sol) for OpenZeppelin's implementation.

Links: https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/libraries/SignatureDecoder.sol#L20

Signature Malleability in recoverKey()

• There is no check for s > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0
• This allows malleable signatures (2 valid signatures for a private key, knowing one can forge the other). This allows for replay attacks. More info: https://medium.com/immunefi/intro-to-cryptography-and-signatures-in-ethereum-2025b6a4a33d
• This is only an issue if nonces are not used either. Since if nonces are used, it would also invalidate the second malleable signature.
• Compare with OpenZeppelin's implementation: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/ec825d8999538f110e572605dc56ef7bf44cc574/contracts/utils/cryptography/ECDSA.sol#L163 for best practice.

Links: https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/libraries/SignatureDecoder.sol#L20