Bug 1261234 - Use isURIPotentiallyTrustworthy to verify the form action.; r?MattN
MozReview-Commit-ID: CqkG54Qj9mm
--- a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
+++ b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
@@ -9,16 +9,22 @@ const STRINGS_URI = "chrome://global/loc
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "devtools",
"resource://devtools/shared/Loader.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "LoginManagerContent",
"resource://gre/modules/LoginManagerContent.jsm");
+XPCOMUtils.defineLazyServiceGetter(this, "gContentSecurityManager",
+ "@mozilla.org/contentsecuritymanager;1",
+ "nsIContentSecurityManager");
+XPCOMUtils.defineLazyServiceGetter(this, "gScriptSecurityManager",
+ "@mozilla.org/scriptsecuritymanager;1",
+ "nsIScriptSecurityManager");
XPCOMUtils.defineLazyGetter(this, "WebConsoleUtils", () => {
return this.devtools.require("devtools/shared/webconsole/utils").Utils;
});
XPCOMUtils.defineLazyGetter(this, "l10n", () => {
return new this.WebConsoleUtils.L10n(STRINGS_URI);
});
this.InsecurePasswordUtils = {
@@ -87,42 +93,48 @@ this.InsecurePasswordUtils = {
// Check if we are on an iframe with insecure src, or inside another
// insecure iframe or document.
if (this._checkForInsecureNestedDocuments(domDoc)) {
this._sendWebConsoleMessage("InsecurePasswordsPresentOnIframe", domDoc);
this._formRootsWarned.set(aForm.rootElement, true);
isSafePage = false;
}
- let isFormSubmitHTTP = false, isFormSubmitHTTPS = false;
+ let isFormSubmitHTTP = false, isFormSubmitSecure = false;
if (aForm.rootElement instanceof Ci.nsIDOMHTMLFormElement) {
- // Note that aForm.action can be a relative path (e.g. "", "/login", "//example.com", etc.)
- // but we don't warn about those since we would have already warned about the form's document
- // not being safe above.
- if (aForm.action.match(/^http:\/\//)) {
- this._sendWebConsoleMessage("InsecureFormActionPasswordsPresent", domDoc);
- this._formRootsWarned.set(aForm.rootElement, true);
+ let uri = Services.io.newURI(aForm.rootElement.action, null, null);
+ let principal = gScriptSecurityManager.getCodebasePrincipal(uri);
+ let host = uri.host;
+
+ if (uri.schemeIs("http")) {
isFormSubmitHTTP = true;
- } else if (aForm.action.match(/^https:\/\//)) {
- isFormSubmitHTTPS = true;
+ if (gContentSecurityManager.isOriginPotentiallyTrustworthy(principal)) {
+ isFormSubmitSecure = true;
+ } else if (isSafePage) {
+ // Only warn about the action if we didn't already warn about the form being insecure.
+ this._sendWebConsoleMessage("InsecureFormActionPasswordsPresent", domDoc);
+ this._formRootsWarned.set(aForm.rootElement, true);
+ }
+ } else {
+ isFormSubmitSecure = true;
}
}
// The safety of a password field determined by the form action and the page protocol
let passwordSafety;
if (isSafePage) {
- if (isFormSubmitHTTPS) {
+ if (isFormSubmitSecure) {
passwordSafety = 0;
} else if (isFormSubmitHTTP) {
passwordSafety = 1;
} else {
passwordSafety = 2;
}
} else {
- if (isFormSubmitHTTPS) {
+ if (isFormSubmitSecure) {
passwordSafety = 3;
} else if (isFormSubmitHTTP) {
passwordSafety = 4;
} else {
passwordSafety = 5;
}
}
--- a/toolkit/components/passwordmgr/test/browser/browser.ini
+++ b/toolkit/components/passwordmgr/test/browser/browser.ini
@@ -1,14 +1,17 @@
[DEFAULT]
support-files =
../formsubmit.sjs
authenticate.sjs
form_basic.html
formless_basic.html
+ form_same_origin_action.html
+ form_cross_origin_insecure_action.html
+ form_cross_origin_secure_action.html
head.js
insecure_test.html
insecure_test_subframe.html
multiple_forms.html
streamConverter_content.sjs
[browser_capture_doorhanger.js]
support-files =
--- a/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordWarning.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordWarning.js
@@ -31,52 +31,64 @@ add_task(function* testInsecurePasswordW
}
}
Services.console.registerListener(messageHandler);
registerCleanupFunction(function() {
Services.console.unregisterListener(messageHandler);
});
for (let [origin, testFile, expectWarnings] of [
- // Form action at 127.0.0.1/localhost is considered as a secure case.
- // There should be no INSECURE_FORM_ACTION warning at 127.0.0.1/localhost.
- // This will be fixed at Bug 1261234.
- ["http://127.0.0.1", "form_basic.html", ["INSECURE_FORM_ACTION"]],
+ ["http://127.0.0.1", "form_basic.html", []],
["http://127.0.0.1", "formless_basic.html", []],
- ["http://example.com", "form_basic.html", ["INSECURE_FORM_ACTION", "INSECURE_PAGE"]],
+ ["http://example.com", "form_basic.html", ["INSECURE_PAGE"]],
["http://example.com", "formless_basic.html", ["INSECURE_PAGE"]],
["https://example.com", "form_basic.html", []],
["https://example.com", "formless_basic.html", []],
+
+ // For a form with customized action link in the same origin.
+ ["http://127.0.0.1", "form_same_origin_action.html", []],
+ ["http://example.com", "form_same_origin_action.html", ["INSECURE_PAGE"]],
+ ["https://example.com", "form_same_origin_action.html", []],
+
+ // For a form with an insecure (http) customized action link.
+ ["http://127.0.0.1", "form_cross_origin_insecure_action.html", ["INSECURE_FORM_ACTION"]],
+ ["http://example.com", "form_cross_origin_insecure_action.html", ["INSECURE_PAGE"]],
+ ["https://example.com", "form_cross_origin_insecure_action.html", ["INSECURE_FORM_ACTION"]],
+
+ // For a form with a secure (https) customized action link.
+ ["http://127.0.0.1", "form_cross_origin_secure_action.html", []],
+ ["http://example.com", "form_cross_origin_secure_action.html", ["INSECURE_PAGE"]],
+ ["https://example.com", "form_cross_origin_secure_action.html", []],
]) {
- let testUrlPath = origin + TEST_URL_PATH + testFile;
- var promiseConsoleMessages = new Promise(resolve => {
+ let testURL = origin + TEST_URL_PATH + testFile;
+ let promiseConsoleMessages = new Promise(resolve => {
warningPatternHandler = function (warning, originMessage) {
ok(warning, "Handling a warning pattern");
- let fullMessage = `[${warning.msg} {file: "${testUrlPath}" line: 0 column: 0 source: "0"}]`;
+ let fullMessage = `[${warning.msg} {file: "${testURL}" line: 0 column: 0 source: "0"}]`;
is(originMessage, fullMessage, "Message full matched:" + originMessage);
let index = expectWarnings.indexOf(warning.key);
- isnot(index, -1, "Found warning: " + warning.key + " for URL:" + testUrlPath);
+ isnot(index, -1, "Found warning: " + warning.key + " for URL:" + testURL);
if (index !== -1) {
// Remove the shown message.
expectWarnings.splice(index, 1);
}
if (expectWarnings.length === 0) {
- info("All warnings are shown. URL:" + testUrlPath);
+ info("All warnings are shown for URL:" + testURL);
resolve();
}
- }
+ };
});
yield BrowserTestUtils.withNewTab({
gBrowser,
- url: testUrlPath
+ url: testURL
}, function*() {
if (expectWarnings.length === 0) {
- info("All warnings are shown. URL:" + testUrlPath);
+ info("All warnings are shown for URL:" + testURL);
return Promise.resolve();
}
return promiseConsoleMessages;
});
// Remove warningPatternHandler to stop handling the matched warning pattern
// and the task should not get any warning anymore.
warningPatternHandler = null;
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/form_cross_origin_insecure_action.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<!-- Simplest form with username and password fields. -->
+<form id="form-basic" action="http://another.domain/custom_action.html">
+ <input id="form-basic-username" name="username">
+ <input id="form-basic-password" name="password" type="password">
+ <input id="form-basic-submit" type="submit">
+</form>
+
+</body></html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/form_cross_origin_secure_action.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<!-- Simplest form with username and password fields. -->
+<form id="form-basic" action="https://another.domain/custom_action.html">
+ <input id="form-basic-username" name="username">
+ <input id="form-basic-password" name="password" type="password">
+ <input id="form-basic-submit" type="submit">
+</form>
+
+</body></html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/form_same_origin_action.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html><html><head><meta charset="utf-8"></head><body>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<!-- Simplest form with username and password fields. -->
+<form id="form-basic" action="./custom_action.html">
+ <input id="form-basic-username" name="username">
+ <input id="form-basic-password" name="password" type="password">
+ <input id="form-basic-submit" type="submit">
+</form>
+
+</body></html>