Skip to content

Commit

Permalink
[BUGFIX] Validate Entry.receiversPubKey for MailboxPayloads
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
julianknutsen committed Nov 14, 2019
1 parent 9ffbcf7 commit bdfe32b
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ public boolean isValidForAddOperation() {
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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ public void isValidForAddOperation_EntryOwnerPayloadReceiverMismatch() throws No
}

// 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();
Expand All @@ -104,8 +102,7 @@ public void isValidForAddOperation_EntryReceiverPayloadReceiverMismatch() throws
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
Expand Down

0 comments on commit bdfe32b

Please sign in to comment.