Skip to content
This repository has been archived by the owner on Nov 21, 2019. It is now read-only.

Withdraw messages can be replayed #12

Open
HarryR opened this issue Nov 16, 2017 · 3 comments
Open

Withdraw messages can be replayed #12

HarryR opened this issue Nov 16, 2017 · 3 comments
Labels

Comments

@HarryR
Copy link
Contributor

HarryR commented Nov 16, 2017

If a user submits a Withdraw message there is nothing preventing others from replaying the same message.

This is a concern if:

  1. The Withdraw fails due to lack of Gas
  2. The replayed message to be processed before the real message (e.g. no guarantee of ordering)

Until this problem is solved it would be possible for an attacker to monitor all rings and retransmit Withdraw messages with no cost of failure.

@HarryR
Copy link
Contributor Author

HarryR commented Nov 16, 2017

According to Matt's input we can bind additional parameters into the signature to prevent replay attacks, or enforce arbitrary logical conditions, see:

algo-fix-fix_1024

At https://github.com/clearmatics/mobius/blob/master/contracts/Ring.sol#L164 the change would be something like, from:

        var hashout = uint256(sha256(commonHashList, hashList)) % GEN_ORDER;

to:

        var hashout = uint256(sha256(commonHashList, hashList, msg.sender)) % GEN_ORDER;

An equivalent change will need to be implemented in the orbital tool.

@rbkhmrcr
Copy link

in the paper we've changed this to the ring signature being formed over the intended recipient address (but this message is not included in the linking tag) -- it also means you can outsource the withdrawal transactions (ie, give it to someone else and include an incentive that pays them if the withdrawal transaction goes through or something) which will mitigate #34 as everyone then has some level of plausible deniability. I'll PR it when I get the chance!

@HarryR
Copy link
Contributor Author

HarryR commented Dec 13, 2017

I've added some of my notes to #22 which covers different withdraw mechanisms, e.g. WithdrawEther would be equivalent to WithdrawEtherTo(msg.sender)

I might be overcomplicating it a bit though.

WithdrawWithIncentiveTo would take an incentive parameter with the amount of Eth/Token to be transferred to the msg.sender in addition to a recipient parameter. But if I add all these things there would be permutations like WithdrawERC223WithIncentiveTo, WithdrawERC223To, WithdrawERC223 etc. and would increase the amount of tests that need writing (unless there's an easy way of generating tests for every permutation).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants