Part IV. 1: Prevent man-in-the-middle attacks which attackers may steal the user's funds by using a revealed secret to forge commit and reveals using higher-priority, competing transactions

[polymorpher]

As described in the TODO list of #5 , to quote from Part IV:

Additionally, during the week of 6/21/2021, a potential man-in-the-middle attack is identified (during a conversation with @ivan-homoliak-sutd), where the attacker may stall the user's commit using an overwhelming amount of spam transactions, and insert the attacker's own commit and reveal in the same block ahead of the user's reveal using an overwhelmingly high amount of gas. A solution to completely prevent this is already found and will be implemented very soon.

It would require quite some efforts from the attacker to make the attack happen, but this is still a high-priority security issue. Here is a summary of the details from the conversation:

Ivan H, [Jun 21, 2021 at 2:32:18 PM]:
I think that I found a security issue in the current commit/reveal protocol. Assume that user just submits reveal tx with OTP and attacker front runs the user's reveal tx: she steals OTP and create malicious commit tx followed by a malicious reveal tx in the same block (by putting higher tx fees than user).

The solution reminds me the 3-stage protocol to replace rootHash in SmartOTPs bended for our purposes: 1) submit h(OTP, params), 2) submit params, 3) submit OTP. However, we do not like 3 txs and it could be reduced to 2 txs: 1) submit h(OTP, params) + params, where store both args in a list. 2) submit OTP + its proof, and find the 1st matching entry in the list. In this case the attacker can only make a later entry in the list than the user, so his entry will not match as the first, when revealing OTP.

Aaron Li:
You are right, there is a problem. It should be fixed. Nonce doesn’t completely solve it.

Assume the attacker is using the same revealed EOTP after the commit block is confirmed, but before the EOTP expires at the end of the time interval. Let’s also assume for simplicity that maxOperationsPerInterval is 1. Each time a commit is revealed, the nonce is incremented by 1 (should be 2, will revise and explain later). If an attacker commits another operation during this time period using nonce 0, he would have to have both his commit and reveal both completed before the user’s reveal is completed. That means he would have to complete both within the same and current block, otherwise the user’s reveal would take precedence and the nonce would already be incremented, which would result the attack to fail.

Now let’s consider the attacker attempting to complete both commit and reveal within the same and current block, which the user’s reveal is pending. This is a problem, since a miner can decide either way. One solution is to prevent the smart contract from allowing commit-reveal both in the same block. But a very sophisticated attacker could in theory fill in so much garbage in the block and prevent the user’s reveal from being confirmed at all, while having his own commit confirmed. With this delay, the attacker may reveal successfully in the next block.

Your method of submitting the parameters at commit time is much more reliable. I will do that, but with a modification so the hash of the parameters are submitted instead of the actual parameters, so we can use a universal commit function for multiple types of operations, and the type and parameters are not revealed during commit.
Actually, it does not prevent the said attack. Because the attacker’s new commit hash would be different. I think the commit hash has to be h(EOTP + nonce) for this to work. Nonce would be 0 in the simplest case which maxOperationsPerInterval=1

Whereupon following the exchange above, Ivan and I spent another ~40 minutes and discussed the commit tracking data structure and mechanisms. Different proposals were made. The next steps are to document these proposals and implement one of them as fast as possible since this is a high priority issue.