Bug 1261234 - Use isURIPotentiallyTrustworthy when checking for insecure form actions.
MozReview-Commit-ID: CqkG54Qj9mm
--- a/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
+++ b/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm
@@ -9,16 +9,19 @@ 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.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 +90,49 @@ 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 baseURI = Services.io.newURI(aForm.rootElement.baseURI, null, null);
+ let uri = Services.io.newURI(aForm.action, null, baseURI);
+ if (uri.schemeIs("http")) {
isFormSubmitHTTP = true;
- } else if (aForm.action.match(/^https:\/\//)) {
- isFormSubmitHTTPS = true;
+ if (gContentSecurityManager.isURIPotentiallyTrustworthy(uri)) {
+ 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_insecurePasswordWarning.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_insecurePasswordWarning.js
@@ -31,52 +31,51 @@ 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"]],
+ // TODO: add test for INSECURE_FORM_ACTION on secure page
+ // TODO: add test for relative 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", []],
]) {
- 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"}]`;
- is(originMessage, fullMessage, "Message full matched:" + originMessage);
+ 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;