add protection against replay between accounts

[JoranHonig]

Hey guys, super cool project!

I really like how you use the Accounts as a mechanism to separate and deal with authentication and replay attacks!

I think there is a slight gap left which would allow for replay attacks. The vector is not insanely significant (it only affects people that re-use keys between accounts), and the repo is still experimental, which is why I'm making this pr out in the open. (It could be cool to add a disclosure policy in the security tab so that people know when it's okay/not okay to submit bugs out in the open)

When a user creates two Accounts with the same private key, then the signatures valid for one, will also be valid for the other.

I think it's possible to harden the implementation by adding the address of the account to the message being signed. Unfortunately this isn't super easy with the current starknet interfaces. In this pr I've drafted a potential solution that creates a kind of "self()" precompile/library contract.

Lmk what you think!

[martriay]

Thanks for this! Definitely a valid point. I was hoping to see the problems that publickey/address decoupling could bring -- I knew it wasn't just features.

Cool hack btw, although to be honest I was hoping to see this solved at the StarkNet level.

Now that I think of it, we don't need the whole call -> get address thing, right? It's just a matter of checking against the internal storage. The only situation I see possible is someone replaying the tx in another contract that has a fake address in its state but I don't think that should be dangerous. What do you think?

[JoranHonig]

I was hoping to see the problems that publickey/address decoupling could bring -- I knew it wasn't just features.

I think that this actually improved the situation. When playing around with cairo & starknet I also built a token implementation and it had the same problem, but the impact was much worse. You could replay transactions between any token instance 🙈.

Cool hack btw, although to be honest I was hoping to see this solved at the StarkNet level.

I completely agree! The current state of manual authentication/ replay prevention is a very big footgun.

Now that I think of it, we don't need the whole call -> get address thing, right? It's just a matter of checking against the internal storage. The only situation I see possible is someone replaying the tx in another contract that has a fake address in its state but I don't think that should be dangerous. What do you think?

Maybe, although, couldn't you still have two Account instances with the same l1 address? (I guess it's a super weird edgecase though).

What's nice about using call -> get address thing is that it creates cross-instance replay mitigation decoupled from the business logic.

[martriay]

What's nice about using call -> get address thing is that it creates cross-instance replay mitigation decoupled from the business logic.

Mhh yeah but it also:

• adds code complexity with extra contract<>contract calls
• it requires an extra deployment
• just checking against storage can be trivially changed to reading a get_self_address() function provided by starknet when it's ready

I think I still lean towards just checking locally, cross-instance risk should be fairly temporal and also super low prio, it would be pretty shady to care about a contract that claims to be on a false address ^^

[JoranHonig]

I 100% agree with all of your points 👍

[martriay]

Thanks again @JoranHonig for raising this. I've implemented the naive storage check in #8.