code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

`CultureIndex.sol:_verifyVoteSignature` does not comply with EIP-712 #658

Closed c4-bot-5 closed 10 months ago

c4-bot-5 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L431

Vulnerability details

The CultureIndex.sol contract implements the voteForManyWithSig and batchVoteForManyWithSig functions, which are used to vote for multiple proposals at once.

Both functions use the internal _verifyVoteSignature function to verify the signature of the voter. This function uses the EIP712 standard to encode the data that is signed by the voter.

431        voteHash = keccak256(abi.encode(VOTE_TYPEHASH, from, pieceIds, nonces[from]++, deadline));

However, the array of pieceIds is not encoded correctly, as according to the EIP-712 specification:

The array values are encoded as the keccak256 hash of the concatenated encodeData of their contents (i.e. the encoding of SomeType[5] is identical to that of a struct containing five members of type SomeType).

Impact

The signature verification is not EIP712 compliant, which will result in issues with integrators.

Tools Used

Manual inspection.

Recommended Mitigation Steps

-       voteHash = keccak256(abi.encode(VOTE_TYPEHASH, from, pieceIds, nonces[from]++, deadline));
+       voteHash = keccak256(abi.encode(VOTE_TYPEHASH, from, keccak256(abi.encodePacked(pieceIds)), nonces[from]++, deadline));

Assessed type

Other

c4-pre-sort commented 10 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #20

c4-judge commented 9 months ago

MarioPoneder marked the issue as satisfactory