thehubbleproject / hubble-contracts

Hubble optimistic rollup
https://thehubbleproject.github.io/docs/
MIT License
133 stars 28 forks source link

Withdraw signature has no nonce #665

Open dmaretskyi opened 2 years ago

dmaretskyi commented 2 years ago

To claim the withdrawn funds to an ethereum address users submit a signature signed with their BLS wallet. Right now that signature only includes the address that the funds are being withdrawn to, and no nonce.

This allows an attacker to re-use the same signature for the next withdrawal from the same account.

https://github.dev/thehubbleproject/hubble-contracts/blob/7058cba1a4e5a6251571f74a6a7f16de5b77e2a7/contracts/WithdrawManager.sol#L98-L98

jacque006 commented 2 years ago

This is similar to the replay attack illustrated in this Solidity example: https://docs.soliditylang.org/en/v0.6.12/solidity-by-example.html?highlight=nonce#what-to-sign

@Marik-D when referring to nonce, I assume you are referring to a data structure that would track nonce's specific to the WithdrawlManager (such as mapping(uint256 => bool) usedNonces from the example above), and not the nonce associated with L1 txns for an ETH address?

dmaretskyi commented 2 years ago

Yes, it doesn't have to be the nonce of the L1 ETH account, just some single-use value. Can be something specific to this withdraw operation.