Open adlrocha opened 2 years ago
Consider also this from @aakoshh:
What if the actor value is wrong? I haven't seen it checked during initialisation. It would mean that some participants never get notifications about results, right?
So what about someone trying to attack like this: Alice, Bob, Charlie and Eve want to participate in a 4 way atomic transaction. Everyone locks their state and give the input CID to Eve. Eve, initialises the atomic swap at the closest common ancestor, but corrupts everyone's actor field except herself. Then she also initialises another swap correctly.
Before the others could submit their outputs, Eve aborts the first atomic swap - since hers is the only correct actor in it, only her subnet gets properly notified; the others go into the dead letters. This notification unlocks her state, but the others don't know this, and they go ahead with the 2nd atomic swap. Eve gets a second notification about the successful swap, but she's already in a state where her state isn't locked.
It seems useless to do this, but the point is that the parent system and the child system are both individually correct, Eve is managing to put them in unexpected states.
Maybe there should be some check that all locked states are only participating in a single atomic swap at any time, in the parent?
Original comment: https://github.com/adlrocha/builtin-actors/pull/13#discussion_r928984624
The SCA of the common parent of all the parties involved in the atomic execution is responsible for orchestrating the protocol. For the execution to succeed, all parties need to submit the same output state, and their CID should match. When the execution succeeds, the SCA propagates the output state in a top-down message to the subnets involved in order to merge the state and unlock all the corresponding locked states of the execution.
Unfortunately, the SCA doesn't perform any check over the outputs submitted, it just verifies that they match, which means that a set of users would be able to collude and initiate an atomic execution to push an invalid state to a subnet through the top-down messages of a successful atomic execution. This is not possible if at least one of the participants involved is honest.
To prevent this attack, we should include checks to verify that the output state only touches parts of the state originally locked by the participants when initializing the atomic execution (users can only lock state that they control in a subnet).