Bug 1418223 - Use the decrypted credit card number to search duplicate records in the storage if master password is disabled. r=steveck draft
authorLuke Chang <lchang@mozilla.com>
Thu, 23 Nov 2017 16:10:36 +0800
changeset 705535 70afa08253f96f250a02cc5396ea54c74d80c547
parent 705442 38f49346a200cc25492236c7b3c536fc835fe031
child 742384 72964c9497650b8471fadd921d6380504af0ca93
push id91501
push userbmo:lchang@mozilla.com
push dateThu, 30 Nov 2017 09:15:34 +0000
reviewerssteveck
bugs1418223
milestone59.0a1
Bug 1418223 - Use the decrypted credit card number to search duplicate records in the storage if master password is disabled. r=steveck MozReview-Commit-ID: 7BihUtjOxvt
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -1654,18 +1654,23 @@ class CreditCards extends AutofillRecord
   getDuplicateGuid(targetCreditCard) {
     let clonedTargetCreditCard = this._clone(targetCreditCard);
     this._normalizeRecord(clonedTargetCreditCard);
     for (let creditCard of this._data) {
       let isDuplicate = this.VALID_FIELDS.every(field => {
         if (!clonedTargetCreditCard[field]) {
           return !creditCard[field];
         }
-        if (field == "cc-number") {
-          return this._getMaskedCCNumber(clonedTargetCreditCard[field]) == creditCard[field];
+        if (field == "cc-number" && creditCard[field]) {
+          if (MasterPassword.isEnabled) {
+            // Compare the masked numbers instead when the master password is
+            // enabled because we don't want to leak the credit card number.
+            return this._getMaskedCCNumber(clonedTargetCreditCard[field]) == creditCard[field];
+          }
+          return clonedTargetCreditCard[field] == MasterPassword.decryptSync(creditCard["cc-number-encrypted"]);
         }
         return clonedTargetCreditCard[field] == creditCard[field];
       });
       if (isDuplicate) {
         return creditCard.guid;
       }
     }
     return null;
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -515,8 +515,50 @@ add_task(async function test_mergeToStor
   do_check_eq(profileStorage.creditCards.getAll()[0]["cc-name"], "Foo Bar");
   do_check_eq(profileStorage.creditCards.getAll()[0]["cc-exp"], "2000-01");
   do_check_eq(profileStorage.creditCards.getAll()[1]["cc-name"], "Foo Bar");
   do_check_eq(profileStorage.creditCards.getAll()[1]["cc-exp"], "2000-01");
 
   // Empty computed fields shouldn't cause any problem.
   do_check_eq(profileStorage.creditCards.mergeToStorage(TEST_CREDIT_CARD_WITH_EMPTY_COMPUTED_FIELD).length, 0);
 });
+
+add_task(async function test_getDuplicateGuid() {
+  let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
+                                                [TEST_CREDIT_CARD_3],
+                                                "creditCards");
+  let guid = profileStorage.creditCards._data[0].guid;
+
+  // Absolutely a duplicate.
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(TEST_CREDIT_CARD_3), guid);
+
+  // Absolutely not a duplicate.
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(TEST_CREDIT_CARD_1), null);
+
+  // Subset shouldn't be treated as a duplicate.
+  let record = Object.assign({}, TEST_CREDIT_CARD_3);
+  delete record["cc-exp-month"];
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(record), null);
+
+  // Superset shouldn't be treated as a duplicate.
+  record = Object.assign({}, TEST_CREDIT_CARD_3);
+  record["cc-name"] = "John Doe";
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(record), null);
+
+  // Numbers with the same last 4 digits shouldn't be treated as a duplicate.
+  record = Object.assign({}, TEST_CREDIT_CARD_3);
+  let last4Digits = record["cc-number"].substr(-4);
+  record["cc-number"] = "000000000000" + last4Digits;
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(record), null);
+
+  // ... However, we treat numbers with the same last 4 digits as a duplicate if
+  // the master password is enabled.
+  let tokendb = Cc["@mozilla.org/security/pk11tokendb;1"].createInstance(Ci.nsIPK11TokenDB);
+  let token = tokendb.getInternalKeyToken();
+  token.reset();
+  token.initPassword("password");
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(record), guid);
+
+  // ... Even though the master password is enabled and the last 4 digits are the
+  // same, an invalid credit card number should never be treated as a duplicate.
+  record["cc-number"] = "************" + last4Digits;
+  do_check_eq(profileStorage.creditCards.getDuplicateGuid(record), null);
+});