Skip to content

Commit

Permalink
fix(plasmaCash): add targetBlock to the transaction signed attributes
Browse files Browse the repository at this point in the history
This security fix will prevent from stealing the money for all cases, where operator should include user block,
but he did not. Not placing transaction on `targetBlock` automatically makes that transaction invalid.
Nobody can can use it as a challenge transaction, because plasma root for `targetBlock` will not match.

LIT-16
  • Loading branch information
DZariusz committed Nov 7, 2018
1 parent c6cf1fd commit 7a241f6
Show file tree
Hide file tree
Showing 12 changed files with 326 additions and 238 deletions.
21 changes: 11 additions & 10 deletions contracts/PlasmaCash.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,24 @@ contract PlasmaCash is Withdrawable, Operable {
emit LogStartExit(msg.sender, _depositId, 0, finalAt);
}

function startTxExit(bytes _txBytes, bytes _proof, bytes _signature, address _spender, uint256 _targetBlock)
function startTxExit(bytes _txBytes, bytes _proof, bytes _signature, address _spender)
public
onlyWithBond
payable {

Transaction.TX memory transaction = Transaction.createTransaction(_txBytes);
validateProofSignaturesAndTxData(_txBytes, _proof, _signature, _spender, _targetBlock);
validateProofSignaturesAndTxData(_txBytes, _proof, _signature, _spender);

require(exits[transaction.depositId][_targetBlock].finalAt == 0, "exit already exists");
require(exits[transaction.depositId][transaction.targetBlock].finalAt == 0, "exit already exists");

Deposit storage depositPtr = deposits[transaction.depositId];
require(depositPtr.amount > 0, "deposit not exists");
require(transaction.newOwner == msg.sender, "you are not the owner");


uint256 finalAt = createExitAndPriority(transaction.depositId, _targetBlock);
uint256 finalAt = createExitAndPriority(transaction.depositId, transaction.targetBlock);

emit LogStartExit(msg.sender, transaction.depositId, _targetBlock, finalAt);
emit LogStartExit(msg.sender, transaction.depositId, transaction.targetBlock, finalAt);
}


Expand All @@ -156,7 +156,7 @@ contract PlasmaCash is Withdrawable, Operable {
}


function challengeExit(bytes _txBytes, bytes _proof, bytes _signature, uint256 _targetBlock)
function challengeExit(bytes _txBytes, bytes _proof, bytes _signature)
public {

Transaction.TX memory transaction = Transaction.createTransaction(_txBytes);
Expand All @@ -171,7 +171,7 @@ contract PlasmaCash is Withdrawable, Operable {
// we will allow users to challenge exit even after challenge time, until someone finalize it
// allow: require(exit.finalAt > block.timestamp, "exit is final, you can't challenge it");

validateProofSignaturesAndTxData(_txBytes, _proof, _signature, exitPtr.exitor, _targetBlock);
validateProofSignaturesAndTxData(_txBytes, _proof, _signature, exitPtr.exitor);

exitPtr.invalid = true;

Expand All @@ -181,18 +181,18 @@ contract PlasmaCash is Withdrawable, Operable {
}


function validateProofSignaturesAndTxData(bytes _txBytes, bytes _proof, bytes _signature, address _signer, uint256 _targetBlock)
function validateProofSignaturesAndTxData(bytes _txBytes, bytes _proof, bytes _signature, address _signer)
public
view
returns (bool) {
Transaction.TX memory transaction = Transaction.createTransaction(_txBytes);
require(transaction.prevTxBlockIndex < chainBlockIndex, "blockchain is the future, but your tx must be from the past");
require(_targetBlock > transaction.prevTxBlockIndex, "invalid targetBlock/prevTxBlockIndex");
require(transaction.targetBlock > transaction.prevTxBlockIndex, "invalid targetBlock/prevTxBlockIndex");

bytes32 hash = Transaction.hashTransaction(transaction);

require(transaction.newOwner != _signer, "preventing sending loop");
require(_proof.verifyProof(blocks[_targetBlock].merkleRoot, hash, transaction.depositId), "MerkleProof.verifyProof() failed");
require(_proof.verifyProof(blocks[transaction.targetBlock].merkleRoot, hash, transaction.depositId), "MerkleProof.verifyProof() failed");
require(Transaction.checkSig(_signer, hash, _signature), "Transaction.checkSig() failed");

return true;
Expand Down Expand Up @@ -241,6 +241,7 @@ contract PlasmaCash is Withdrawable, Operable {
}



// helpers methods - just for testing, can be removed for release


Expand Down
8 changes: 5 additions & 3 deletions contracts/lib/Transaction.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,20 @@ library Transaction {
uint256 depositId;
uint256 prevTxBlockIndex;
address newOwner;
uint256 targetBlock;
}

/// @dev this is ETH prefix, so we can be sure, data was signed in EVM
bytes constant ETH_PREFIX = "\x19Ethereum Signed Message:\n32";

function createTransaction(bytes rlp) internal pure returns (TX memory) {
RLP.RLPItem[] memory txList = rlp.toRLPItem().toList();
require(txList.length == 3, "txList.length == 3");
require(txList.length == 4, "txList.length == 4");
return TX({
depositId: txList[0].toUint(),
prevTxBlockIndex: txList[1].toUint(),
newOwner: txList[2].toAddress()
newOwner: txList[2].toAddress(),
targetBlock: txList[3].toUint()
});
}

Expand All @@ -36,6 +38,6 @@ library Transaction {
internal
pure
returns (bytes32) {
return keccak256(abi.encodePacked(_tx.depositId, _tx.prevTxBlockIndex, _tx.newOwner));
return keccak256(abi.encodePacked(_tx.depositId, _tx.prevTxBlockIndex, _tx.newOwner, _tx.targetBlock));
}
}
Loading

0 comments on commit 7a241f6

Please sign in to comment.