mobius icon indicating copy to clipboard operation
mobius copied to clipboard

Withdraw messages can be replayed

Open HarryR opened this issue 6 years ago • 3 comments

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 avatar Nov 16 '17 13:11 HarryR

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.

HarryR avatar Nov 16 '17 16:11 HarryR

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!

rbkhmrcr avatar Dec 13 '17 08:12 rbkhmrcr

I've added some of my notes to https://github.com/clearmatics/mobius/issues/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).

HarryR avatar Dec 13 '17 11:12 HarryR