From bdfe32bd18d246fb947700baf4194957fad38f96 Mon Sep 17 00:00:00 2001 From: Julian Knutsen Date: Thu, 14 Nov 2019 10:19:28 -0800 Subject: [PATCH] [BUGFIX] Validate Entry.receiversPubKey for MailboxPayloads The remove code checks to ensure these fields match, but the add code never did. This could lead to a situation where a MailboxStoragePayload could be added, but never removed. --- .../p2p/storage/payload/ProtectedMailboxStorageEntry.java | 7 +++++++ .../storage/payload/ProtectedMailboxStorageEntryTest.java | 5 +---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntry.java b/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntry.java index f0d51ef610..feaa7b19dd 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntry.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntry.java @@ -94,6 +94,13 @@ public class ProtectedMailboxStorageEntry extends ProtectedStorageEntry { return false; MailboxStoragePayload mailboxStoragePayload = this.getMailboxStoragePayload(); + + // Verify the Entry.receiversPubKey matches the Payload.ownerPubKey. This is a requirement for removal + if (!mailboxStoragePayload.getOwnerPubKey().equals(this.receiversPubKey)) { + log.debug("Entry receiversPubKey does not match payload owner which is a requirement for adding MailboxStoragePayloads"); + return false; + } + boolean result = mailboxStoragePayload.getSenderPubKeyForAddOperation() != null && mailboxStoragePayload.getSenderPubKeyForAddOperation().equals(this.getOwnerPubKey()); diff --git a/p2p/src/test/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntryTest.java b/p2p/src/test/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntryTest.java index b97850fc38..74d466aa14 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntryTest.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntryTest.java @@ -94,8 +94,6 @@ public class ProtectedMailboxStorageEntryTest { } // TESTCASE: validForAddOperation() should fail if Entry.receiversPubKey and Payload.ownerPubKey don't match - // XXXBUGXXX: The current code doesn't validate this mismatch, but it would create an added payload that could never - // be removed since the remove code requires Entry.receiversPubKey == Payload.ownerPubKey @Test public void isValidForAddOperation_EntryReceiverPayloadReceiverMismatch() throws NoSuchAlgorithmException, CryptoException { KeyPair senderKeys = TestUtils.generateKeyPair(); @@ -104,8 +102,7 @@ public class ProtectedMailboxStorageEntryTest { MailboxStoragePayload mailboxStoragePayload = buildMailboxStoragePayload(senderKeys.getPublic(), receiverKeys.getPublic()); ProtectedStorageEntry protectedStorageEntry = buildProtectedMailboxStorageEntry(mailboxStoragePayload, senderKeys, senderKeys.getPublic(), 1); - // should be assertFalse - Assert.assertTrue(protectedStorageEntry.isValidForAddOperation()); + Assert.assertFalse(protectedStorageEntry.isValidForAddOperation()); } // TESTCASE: validForAddOperation() should fail if the signature isn't valid