semiotic-ai / timeline-aggregation-protocol-contracts

Smart contracts for timeline aggregation protocol
Apache License 2.0
3 stars 1 forks source link

Analyze replay attack for authorizedSigner #18

Closed ColePBryan closed 1 year ago

ColePBryan commented 1 year ago
          I think the idea is to add a method to revoke an authorized signer right? If so then the proofs here allow for a replay attack, since the proof is public once posted you could re-authorize a signer even after the sender has revoked. 

We can fix that by adding a nonce to the signed message. The GRT contract implements a similar thing for permits here: https://github.com/graphprotocol/contracts/blob/dev/contracts/token/GraphToken.sol#L110 Essentially we add a mapping storing nonces per sender. Then part of the signed message is the current nonce. After validating the signed message the nonce gets incremented so the same signed message can not be re-used

_Originally posted by @tmigone in https://github.com/semiotic-ai/timeline-aggregation-protocol-contracts/pull/4#discussion_r1239865185_

reply thread:

@ColePBryan ColePBryan I'm not sure if this is needed. The only one who would be able to revoke a signer is the sender. The reason for revoking a signer is if the sender feels the signer has been compromised. If that is the case what would be the reason for re-adding the signer? They could have just kept the signer in the first place.

@tmigone tmigone For the sender there is no reason, but if I'm an attacker that got hold of signer keys I can reuse the proof that was originally submitted to re-add the signer as a valid one. Then I could forge my own RAVs and 🎄

@ColePBryan ColePBryan Would an attacker be able to spoof msg.sender to be set to the senders address? Otherwise, I don't see how the attacker getting ahold of the proof would help them. The proof check uses the msg.sender for the call to authorizeSigner. They would only be able to add the signer as authorized to their own address.

@tmigone tmigone Ahh good point. Yeah agreed then, there isn't much harm that could be done.

Even then, I just realized that the verifyAuthorizedSignerProof uses msg.sender as the message, so even with a stolen proof you couldn't get past the proof verification.

ColePBryan commented 1 year ago

@tmigone are we good to close this one?

tmigone commented 1 year ago

Yes!