code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Missing Nonce and ChainId may lead to signature replay attack #336

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L427-L437 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L458-L472 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L494-L504 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1093-L1104

Vulnerability details

Impact

Missing nonce in several permit function may lead to signature replay attacks which allow an attacker to replay a previous transaction by copying its signature and passing the validation check.

Proof of Concept

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L427-L437

Tools Used

Manual Review

Recommended Mitigation Steps

For missing nonce, smart contracts must:

For cross-chain signature replay: a valid signature that was used on one chain could be copied by an attacker and propagated onto another chain, where it would also be valid for the same user & contract address! To prevent cross-chain signature replay attacks, smart contracts must validate the signature using the chain_id, and users must include the chain_id in the message to be signed

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 1 year ago

The permit signature uses both a nonce and the chainId