sherlock-audit / 2022-10-mover-judging

1 stars 0 forks source link

caventa - Nonces not used in signed data #57

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

caventa

medium

Nonces not used in signed data

Summary

Nonces are not used in the signature checks

Vulnerability Detail

A nonce can prevent an old value from being used when a new value exists. Without one, two transactions submitted in one order can appear in a block in a different order

Impact

If a user is attacked, then tries to change the recipient address to a more secure address, initially chooses an insecure compromised one, but immediately notices the problem, then re-submits as a different, uncompromised address, a malicious miner can change the order of the transactions, so the insecure one is the one that ends up taking effect, letting the attacker transfer the funds

Code Snippet

https://github.com/sherlock-audit/2022-10-mover/blob/main/cardtopup_contract/contracts/HardenedTopupProxy.sol#L508-L514

Tool used

Manual Review

Recommendation

Include a nonce in what is signed

McMannaman commented 2 years ago

The description does not show what is the issue. The nonce is not included in the referenced code structure because we don't care about the nonce at this stage: the data checked in MPT proof has already to be in existing block, for which EVM has non-zero block hash available to on-chain check against as the source of truth. So no reordering can occur, or if user has several transactions in mempool -- that's his right, they may be executed. Not an issue or the description is lacking detail.