Bug 1427624 - Pass empty value rather than apparently-obfuscated username in Save Password doorhanger. r=MattN
authorSam Foster <sfoster@mozilla.com>
Tue, 02 Apr 2019 17:32:52 +0000
changeset 467635 41f4b3d8de33b908948fc3ae38c6630511fe823a
parent 467634 90c5adf892a17e36c82711e4c291bfe66c19d213
child 467636 c15f491f127a206fbd88c4937515ff11fe7484f9
push id35806
push userrgurzau@mozilla.com
push dateWed, 03 Apr 2019 04:07:39 +0000
treeherdermozilla-central@45808ab18609 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1427624
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1427624 - Pass empty value rather than apparently-obfuscated username in Save Password doorhanger. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D24824
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/mochitest/mochitest.ini
toolkit/components/passwordmgr/test/mochitest/test_munged_username.html
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -1079,16 +1079,21 @@ var LoginManagerContent = {
     var [usernameField, newPasswordField, oldPasswordField] =
           this._getFormFields(form, true, recipes);
 
     // Need at least 1 valid password field to do anything.
     if (newPasswordField == null) {
       return;
     }
 
+    if (usernameField && usernameField.value.match(/[•\*]{3,}/)) {
+      log(`usernameField.value "${usernameField.value}" looks munged, setting to null`);
+      usernameField = null;
+    }
+
     // Check for autocomplete=off attribute. We don't use it to prevent
     // autofilling (for existing logins), but won't save logins when it's
     // present and the storeWhenAutocompleteOff pref is false.
     // XXX spin out a bug that we don't update timeLastUsed in this case?
     if ((this._isAutocompleteDisabled(form) ||
          this._isAutocompleteDisabled(usernameField) ||
          this._isAutocompleteDisabled(newPasswordField) ||
          this._isAutocompleteDisabled(oldPasswordField)) &&
--- a/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
+++ b/toolkit/components/passwordmgr/test/mochitest/mochitest.ini
@@ -92,16 +92,19 @@ skip-if = toolkit == 'android' && debug 
 [test_input_events_for_identical_values.html]
 [test_master_password.html]
 scheme = https
 skip-if = os != 'mac' # Tests desktop prompts and bug 1333264
 support-files =
   chrome_timeout.js
   subtst_master_pass.html
 [test_maxlength.html]
+[test_munged_username.html]
+scheme = https
+skip-if = toolkit == 'android' # bug 1527403
 [test_autocomplete_new_password.html]
 scheme = https
 skip-if = toolkit == 'android' # autocomplete
 [test_one_doorhanger_per_un_pw.html]
 scheme = https
 skip-if = toolkit == 'android' # bug 1535505
 [test_onsubmit_value_change.html]
 [test_passwords_in_type_password.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/mochitest/test_munged_username.html
@@ -0,0 +1,268 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test handling of possibly-manipulated username values</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="text/javascript" src="../../../satchel/test/satchel_common.js"></script>
+  <script src="pwmgr_common.js"></script>
+  <link rel="stylesheet" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<script type="application/javascript">
+const LMCBackstagePass = SpecialPowers.Cu.import("resource://gre/modules/LoginManagerContent.jsm");
+const { LoginManagerContent } = LMCBackstagePass;
+
+let readyPromise = registerRunTests();
+
+function add2logins() {
+  runInParent(function initLogins() {
+    const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+    Services.logins.removeAllLogins();
+
+    let login1 = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
+    login1.init("https://example.org", "https://example.org", null, "real••••user", "pass1", "", "");
+
+    let login2 = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
+    login2.init("https://example.org", "https://example.org", null, "user2", "pass2", "", "");
+
+    Services.logins.addLogin(login1);
+    Services.logins.addLogin(login2);
+  });
+}
+
+function addSingleLogin() {
+  runInParent(function initWithSingleLogin() {
+    const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+    Services.logins.removeAllLogins();
+
+    let login1 = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
+    login1.init("https://example.org", "https://example.org", null, "real••••user", "pass1", "", "");
+    Services.logins.addLogin(login1);
+  });
+}
+
+let loadPromise = new Promise(resolve => {
+  document.addEventListener("DOMContentLoaded", () => {
+    document.getElementById("loginFrame").addEventListener("load", (evt) => {
+      resolve();
+    });
+  });
+});
+
+async function loadFormIntoIframe(origin, html) {
+  let loginFrame = document.getElementById("loginFrame");
+  let loadedPromise = new Promise((resolve) => {
+    loginFrame.addEventListener("load", function() {
+      resolve();
+    }, {once: true});
+  });
+  loginFrame.src = origin + "/tests/toolkit/components/passwordmgr/test/mochitest/blank.html";
+  await loadedPromise;
+  let frameDoc = SpecialPowers.wrap(loginFrame.contentWindow).document;
+  // eslint-disable-next-line no-unsanitized/property
+  frameDoc.documentElement.innerHTML = html;
+  // Wait for the form to be processed before trying to submit.
+  await promiseFormsProcessed();
+  return frameDoc;
+}
+
+add_task(async function setup() {
+  info("Waiting for setup and page and frame loads");
+  await readyPromise;
+  await loadPromise;
+});
+
+const DEFAULT_ORIGIN = "https://example.com";
+const ORG_ORIGIN = "https://example.org";
+
+const TESTCASES = [
+  {
+    testName: "test_middleAsteriskMaskedUsername",
+    username: "so***ne",
+    expected: null,
+  },
+  {
+    testName: "test_middleBulletMaskedUsername",
+    username: "so•••ne",
+    expected: null,
+  },
+  {
+    testName: "test_startAsteriskMaskedUsername",
+    username: "***eone",
+    expected: null,
+  },
+  {
+    testName: "test_startBulletMaskedUsername",
+    username: "•••eone",
+    expected: null,
+  },
+  {
+    testName: "test_endAsteriskMaskedUsername",
+    username: "some***",
+    expected: null,
+  },
+  {
+    testName: "test_endBulletMaskedUsername",
+    username: "some•••",
+    expected: null,
+  },
+  {
+    testName: "test_okAsteriskUsername",
+    username: "obelixand*",
+    expected: "obelixand*",
+  },
+  {
+    testName: "test_okBulletUsername",
+    username: "•time",
+    expected: "•time",
+  },
+  {
+    testName: "test_okAsteriskUsername2",
+    username: "**username**",
+    expected: "**username**",
+  },
+  {
+    testName: "test_okBulletUsername2",
+    username: "••user••",
+    expected: "••user••",
+  },
+];
+
+/**
+ * @return {Promise} resolving when form submission was processed.
+ */
+function getSubmitMessage() {
+  info("getSubmitMessage");
+  return new Promise((resolve, reject) => {
+    PWMGR_COMMON_PARENT.addMessageListener("formSubmissionProcessed", function processed(...args) {
+      info("got formSubmissionProcessed");
+      PWMGR_COMMON_PARENT.removeMessageListener("formSubmissionProcessed", processed);
+      resolve(...args);
+    });
+  });
+}
+
+add_task(async function test_new_logins() {
+  for (let tc of TESTCASES) {
+    info("Starting testcase: " + JSON.stringify(tc));
+    let frameDoc = await loadFormIntoIframe(DEFAULT_ORIGIN, `<form id="form1" onsubmit="return false;">
+      <input  type="text"       name="uname" value="${tc.username}">
+      <input  type="password"   name="pword" value="thepassword">
+      <button type="submit" id="submitBtn">Submit</button>
+    </form>`);
+    is(frameDoc.querySelector("[name='uname']").value, tc.username, "Checking for filled username");
+
+    // Check data sent via PasswordManager:onFormSubmit
+    let processedPromise = getSubmitMessage();
+    frameDoc.getElementById("submitBtn").click();
+
+    let submittedResult = await processedPromise;
+    info("Got submittedResult: " + JSON.stringify(submittedResult));
+
+    if (tc.expected === null) {
+      is(submittedResult.usernameField, tc.expected, "Check usernameField");
+    } else {
+      is(submittedResult.usernameField.value, tc.expected, "Check usernameField");
+    }
+  }
+});
+
+add_task(async function test_no_autofill_munged_username_matching_password() {
+  // run this test with 2 matching logins from this origin so we don't autofill
+  await add2logins();
+
+  let allLogins = await LoginManager.getAllLogins();
+  let matchingLogins = allLogins.filter(l => l.hostname == ORG_ORIGIN);
+  is(matchingLogins.length, 2, "Expected number of matching logins");
+
+  let bulletLogin = matchingLogins.find(l => l.username == "real••••user");
+  ok(bulletLogin, "Found the real••••user login");
+
+  let timesUsed = bulletLogin.timesUsed;
+  let guid = bulletLogin.guid;
+
+  let frameDoc = await loadFormIntoIframe(ORG_ORIGIN, `<form id="form1" onsubmit="return false;">
+    <input  type="text"       name="uname" value="">
+    <input  type="password"   name="pword" value="">
+    <button type="submit" id="submitBtn">Submit</button>
+  </form>`);
+  is(frameDoc.querySelector("[name='uname']").value, "", "Check username didn't get autofilled");
+  frameDoc.querySelector("[name='uname']").setUserInput("real••••user");
+  frameDoc.querySelector("[name='pword']").setUserInput("pass1");
+
+  // we shouldn't get the save password doorhanger...
+  let popupShownPromise = promiseNoUnexpectedPopupShown();
+
+  // Check data sent via PasswordManager:onFormSubmit
+  let processedPromise = getSubmitMessage();
+  frameDoc.getElementById("submitBtn").click();
+
+  let submittedResult = await processedPromise;
+  info("Got submittedResult: " + JSON.stringify(submittedResult));
+
+  is(submittedResult.usernameField, null, "Check usernameField");
+
+  let updatedLogins = await LoginManager.getAllLogins();
+  let updatedLogin = updatedLogins.find(l => l.guid == guid);
+  ok(updatedLogin, "Got the login via guid");
+  is(updatedLogin.timesUsed, timesUsed + 1, "timesUsed was incremented");
+
+  await popupShownPromise;
+});
+
+
+add_task(async function test_autofill_munged_username_matching_password() {
+  // only a single matching login so we autofill the username
+  await addSingleLogin();
+
+  let allLogins = await LoginManager.getAllLogins();
+  let matchingLogins = allLogins.filter(l => l.hostname == ORG_ORIGIN);
+  is(matchingLogins.length, 1, "Expected number of matching logins");
+
+  info("matched login: " + matchingLogins[0].username);
+  let bulletLogin = matchingLogins.find(l => l.username == "real••••user");
+  ok(bulletLogin, "Found the real••••user login");
+
+  let timesUsed = bulletLogin.timesUsed;
+  let guid = bulletLogin.guid;
+
+  let frameDoc = await loadFormIntoIframe(ORG_ORIGIN, `<form id="form1" onsubmit="return false;">
+    <input  type="text"       name="uname" value="">
+    <input  type="password"   name="pword" value="">
+    <button type="submit" id="submitBtn">Submit</button>
+  </form>`);
+  is(frameDoc.querySelector("[name='uname']").value, "real••••user", "Check username did get autofilled");
+  frameDoc.querySelector("[name='pword']").setUserInput("pass1");
+
+  // we shouldn't get the save/update password doorhanger as it didn't change
+  let popupShownPromise =  promiseNoUnexpectedPopupShown();
+
+  // Check data sent via PasswordManager:onFormSubmit
+  let processedPromise = getSubmitMessage();
+  frameDoc.getElementById("submitBtn").click();
+
+  let submittedResult = await processedPromise;
+  info("Got submittedResult: " + JSON.stringify(submittedResult));
+
+  is(submittedResult.usernameField, null, "Check usernameField");
+
+  let updatedLogins = await LoginManager.getAllLogins();
+  let updatedLogin = updatedLogins.find(l => l.guid == guid);
+  ok(updatedLogin, "Got the login via guid");
+  is(updatedLogin.timesUsed, timesUsed + 1, "timesUsed was incremented");
+
+  await popupShownPromise;
+});
+
+</script>
+
+<p id="display"></p>
+
+<div id="content">
+  <iframe id="loginFrame" src="https://example.com/tests/toolkit/components/passwordmgr/test/mochitest/blank.html"></iframe>
+</div>
+<pre id="test"></pre>
+</body>
+</html>