loomnetwork / plasma-cash

Plasma Cash Contract & Client. ERC721, ERC20, and ETH compatible
https://loomx.io/developers
Other
271 stars 62 forks source link

(WIP) Challenge before bug fix & potential startExit fix? #77

Closed aupiff closed 6 years ago

aupiff commented 6 years ago

As I was working on a test to demonstrate a challengeBefore bug I brought up to @gakonst, I think I discovered an exploit where anyone can submit a valid exit for another user and then claim their coin.

It's late, sorry for the messy WIP PR...

gakonst commented 6 years ago

Fixed the startExit but by requiring that the msg.sender when initiating an exit is the exiting transaction's owner.

In the context of delegated exits this can be slightly modified so that the exiting transaction's owner is stored in pushExit instead of msg.sender. This however means that anybody could start exiting coins of other users which would be a usability problem. As a mitigation to this, a user could approve a list of users and only allow them to exit their coin (in addition to the user himself of course). This is an additional feature and I do not consider it high priority just yet, so the sanity check in the beginning of the exit is sufficient.

As for the challengeBefore bug, renamed the variable responses to challenges and made it a struct which stores the challenge owner as well as the block at which a challenge was initiated.

For a valid response, a user must provide a spend signed by the challenge's owner which happened at a later block than the challenge's block, which was not the case before.