diff --git a/raiden/smart_contracts/NettingChannelLibrary.sol b/raiden/smart_contracts/NettingChannelLibrary.sol index 9908ba4ffd..84ebf76ca3 100644 --- a/raiden/smart_contracts/NettingChannelLibrary.sol +++ b/raiden/smart_contracts/NettingChannelLibrary.sol @@ -126,6 +126,7 @@ library NettingChannelLibrary { bytes memory transfer_raw; uint64 nonce; address transfer_address; + address recipient; bytes32 locksroot; uint256 transferred_amount; @@ -161,13 +162,18 @@ library NettingChannelLibrary { // by the closing node Participant storage counterparty = self.participants[counterparty_index]; - (nonce, locksroot, transferred_amount) = decodeTransfer(transfer_raw); + (nonce, recipient, locksroot, transferred_amount) = decodeTransfer(transfer_raw); // only accept messages with a valid nonce if (!isValidNonce(self, nonce)) { throw; } + // the registered message recipient should be the closing party + if (recipient != self.closing_address) { + throw; + } + counterparty.nonce = nonce; counterparty.locksroot = locksroot; counterparty.transferred_amount = transferred_amount; @@ -183,6 +189,7 @@ library NettingChannelLibrary { stillTimeout(self) { address transfer_address; + address recipient; bytes32 locksroot; bytes memory transfer_raw; uint256 transferred_amount; @@ -215,13 +222,21 @@ library NettingChannelLibrary { // counterparty closer_index = 1 - caller_index; - (nonce, locksroot, transferred_amount) = decodeTransfer(transfer_raw); + (nonce, recipient, locksroot, transferred_amount) = decodeTransfer(transfer_raw); // only accept messages with a valid nonce if (!isValidNonce(self, nonce)) { throw; } + // the registered recipient in the message should be us + // Note: could have taken msg.sender here but trying to be future-proof + // for when we allow third party updates + Participant storage updating_party = self.participants[caller_index]; + if (updating_party.node_address != recipient) { + throw; + } + self.participants[closer_index].nonce = nonce; self.participants[closer_index].locksroot = locksroot; self.participants[closer_index].transferred_amount = transferred_amount; @@ -444,7 +459,7 @@ library NettingChannelLibrary { function decodeTransfer(bytes transfer_raw) internal - returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount) + returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount) { uint cmdid = uint(transfer_raw[0]); @@ -461,7 +476,7 @@ library NettingChannelLibrary { function decodeDirectTransfer(bytes memory message) private - returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount) + returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount) { // size of the raw message without the signature if (message.length != 124) { @@ -479,6 +494,7 @@ library NettingChannelLibrary { // [92:124] optional_locksroot assembly { nonce := mload(add(message, 12)) + recipient := mload(add(message, 60)) transferred_amount := mload(add(message, 92)) locksroot := mload(add(message, 124)) } @@ -486,7 +502,7 @@ library NettingChannelLibrary { function decodeMediatedTransfer(bytes memory message) private - returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount) + returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount) { // size of the raw message without the signature if (message.length != 268) { @@ -510,6 +526,7 @@ library NettingChannelLibrary { // [236:268] fee assembly { nonce := mload(add(message, 12)) + recipient := mload(add(message, 68)) locksroot := mload(add(message, 140)) transferred_amount := mload(add(message, 204)) } @@ -517,7 +534,7 @@ library NettingChannelLibrary { function decodeRefundTransfer(bytes memory message) private - returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount) + returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount) { // size of the raw message without the signature if (message.length != 196) { @@ -538,6 +555,7 @@ library NettingChannelLibrary { // [164:196] hashlock assembly { nonce := mload(add(message, 12)) + recipient := mload(add(message, 68)) locksroot := mload(add(message, 100)) transferred_amount := mload(add(message, 132)) } diff --git a/raiden/tests/smart_contracts/netting_channel/DecoderTester.sol b/raiden/tests/smart_contracts/netting_channel/DecoderTester.sol index 45f6dca5aa..5b08981d89 100644 --- a/raiden/tests/smart_contracts/netting_channel/DecoderTester.sol +++ b/raiden/tests/smart_contracts/netting_channel/DecoderTester.sol @@ -11,7 +11,7 @@ contract DecoderTester { return NettingChannelLibrary.getTransferRawAddress(signed_transfer); } - function decodeTransfer(bytes transfer_raw) returns (uint64 nonce, bytes32 locksroot, uint256 transferred_amount) { + function decodeTransfer(bytes transfer_raw) returns (uint64 nonce, address recipient, bytes32 locksroot, uint256 transferred_amount) { return NettingChannelLibrary.decodeTransfer(transfer_raw); } } diff --git a/raiden/tests/smart_contracts/netting_channel/test_close.py b/raiden/tests/smart_contracts/netting_channel/test_close.py index dee8df636c..365f3afbe1 100644 --- a/raiden/tests/smart_contracts/netting_channel/test_close.py +++ b/raiden/tests/smart_contracts/netting_channel/test_close.py @@ -81,11 +81,12 @@ def test_close_accepts_only_transfer_from_participants(tester_channels, private_ """ Close must not accept a transfer from a non participant. """ pkey0, _, nettingchannel, channel0, _ = tester_channels[0] nonparticipant_key = private_keys[2] + opened_block = nettingchannel.opened(sender=pkey0) # make a transfer where pkey0 is the target transfer_nonparticipant = DirectTransfer( identifier=1, - nonce=1, + nonce=1 + (opened_block * (2 ** 32)), token=channel0.token_address, transferred_amount=10, recipient=channel0.our_address, @@ -102,6 +103,30 @@ def test_close_accepts_only_transfer_from_participants(tester_channels, private_ nettingchannel.close(transfer_nonparticipant_data, sender=pkey0) +@pytest.mark.parametrize('number_of_nodes', [3]) +def test_close_wrong_recipient(tester_channels, private_keys): + """ Close must not accept a transfer aimed at a non recipient. """ + pkey0, pkey1, nettingchannel, channel0, _ = tester_channels[0] + opened_block = nettingchannel.opened(sender=pkey0) + nonparticipant_address = privatekey_to_address(private_keys[2]) + + # make a transfer where the recipient is totally wrong + transfer_wrong_recipient = DirectTransfer( + identifier=1, + nonce=1 + (opened_block * (2 ** 32)), + token=channel0.token_address, + transferred_amount=10, + recipient=nonparticipant_address, + locksroot='', + ) + + transfer_wrong_recipient.sign(PrivateKey(pkey1), privatekey_to_address(pkey1)) + transfer_wrong_recipient_data = str(transfer_wrong_recipient.packed().data) + + with pytest.raises(TransactionFailed): + nettingchannel.close(transfer_wrong_recipient_data, sender=pkey0) + + def test_close_called_multiple_times(tester_state, tester_nettingcontracts): """ A channel can be closed only once. """ pkey0, pkey1, nettingchannel = tester_nettingcontracts[0] diff --git a/raiden/tests/smart_contracts/netting_channel/test_decoder_netting_channel.py b/raiden/tests/smart_contracts/netting_channel/test_decoder_netting_channel.py index 93c335d31f..0dc470f8ae 100644 --- a/raiden/tests/smart_contracts/netting_channel/test_decoder_netting_channel.py +++ b/raiden/tests/smart_contracts/netting_channel/test_decoder_netting_channel.py @@ -34,11 +34,13 @@ def assert_decoder_results(message, decoder): ( nonce_decoded, + recipient_address_decoded, locksroot_decoded, transferred_amount_decoded ) = decoder.decodeTransfer(transfer_raw) assert message.nonce == nonce_decoded + assert message.recipient == address_decoder(recipient_address_decoded) assert message.transferred_amount == transferred_amount_decoded assert message.locksroot == locksroot_decoded diff --git a/raiden/tests/smart_contracts/netting_channel/test_updatetransfer.py b/raiden/tests/smart_contracts/netting_channel/test_updatetransfer.py index 6c953c471d..cc9cea0ff2 100644 --- a/raiden/tests/smart_contracts/netting_channel/test_updatetransfer.py +++ b/raiden/tests/smart_contracts/netting_channel/test_updatetransfer.py @@ -83,11 +83,12 @@ def test_update_must_fail_with_a_nonparticipant_transfer(tester_channels, privat """ updateTransfer must not accept a transfer from a non participant. """ pkey0, pkey1, nettingchannel, channel0, channel1 = tester_channels[0] nonparticipant_key = private_keys[2] + opened_block = nettingchannel.opened(sender=pkey0) # make a transfer where pkey1 is the target transfer_nonparticipant = DirectTransfer( identifier=1, - nonce=1, + nonce=1 + (opened_block * (2 ** 32)), token=channel0.token_address, transferred_amount=10, recipient=channel1.our_address, @@ -106,6 +107,35 @@ def test_update_must_fail_with_a_nonparticipant_transfer(tester_channels, privat nettingchannel.updateTransfer(transfer_nonparticipant_data, sender=pkey1) +@pytest.mark.parametrize('number_of_nodes', [3]) +def test_update_must_fail_with_a_wrong_recipient(tester_channels, private_keys): + """ updateTransfer must not accept a transfer from a non participant. """ + pkey0, pkey1, nettingchannel, channel0, channel1 = tester_channels[0] + opened_block = nettingchannel.opened(sender=pkey0) + nonparticipant_address = privatekey_to_address(private_keys[2]) + + # make a transfer where pkey1 is the target + transfer_wrong_recipient = DirectTransfer( + identifier=1, + nonce=1 + (opened_block * (2 ** 32)), + token=channel0.token_address, + transferred_amount=10, + recipient=nonparticipant_address, + locksroot='', + ) + + our_address = privatekey_to_address(pkey0) + our_sign_key = PrivateKey(pkey0) + + transfer_wrong_recipient.sign(our_sign_key, our_address) + transfer_wrong_recipient_data = str(transfer_wrong_recipient.packed().data) + + nettingchannel.close('', sender=pkey0) + + with pytest.raises(TransactionFailed): + nettingchannel.updateTransfer(transfer_wrong_recipient_data, sender=pkey1) + + def test_update_called_multiple_times_same_transfer(tester_channels): """ updateTransfer can be called only once. """ pkey0, pkey1, nettingchannel, channel0, channel1 = tester_channels[0] diff --git a/raiden/tests/smart_contracts/netting_channel/test_withdraw.py b/raiden/tests/smart_contracts/netting_channel/test_withdraw.py index 4bf57c3d9e..e42868563f 100644 --- a/raiden/tests/smart_contracts/netting_channel/test_withdraw.py +++ b/raiden/tests/smart_contracts/netting_channel/test_withdraw.py @@ -362,6 +362,7 @@ def test_withdraw_fails_with_partial_merkle_proof( direct_transfer = make_direct_transfer( nonce=nonce, locksroot=merkle_tree.merkleroot, + recipient=privatekey_to_address(pkey1) ) address = privatekey_to_address(pkey0) @@ -412,6 +413,7 @@ def test_withdraw_tampered_merkle_proof(tree, tester_channels, tester_state, set direct_transfer = make_direct_transfer( nonce=nonce, locksroot=merkle_tree.merkleroot, + recipient=privatekey_to_address(pkey1) ) address = privatekey_to_address(pkey0) @@ -474,6 +476,7 @@ def test_withdraw_tampered_lock_amount( nonce=nonce, locksroot=merkle_tree.merkleroot, token=tester_token.address, + recipient=privatekey_to_address(pkey1) ) address = privatekey_to_address(pkey0)