Closed Bug 1726621 (CVE-2021-38497) Opened 3 years ago Closed 3 years ago

Show form validationMessage on any website with window.open

Categories

(Toolkit :: UI Widgets, defect)

Firefox 91
Desktop
All
defect

Tracking

()

VERIFIED FIXED
94 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 93+ verified
firefox91 --- wontfix
firefox92 --- wontfix
firefox93 + verified
firefox94 + verified

People

(Reporter: sourc7, Assigned: Gijs)

References

(Regression)

Details

(Keywords: csectype-spoof, regression, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main93+][adv-esr91.2+])

Attachments

(4 files, 1 obsolete file)

After call reportValidity() then open target URL with window.open() the reportValidity validationMessage will appear on the target website, it also looks to be called from the target website which is more convincing.

As the validationMessage appear on trusted website, users are more likely to trust to follow the message instructions.

Mozregression show it is regression of Bug 1684792, open form validation popup anchored at screen coordinate as datetime picker and select do so that it is positioned correctly in out of process iframes

Affected version:

  • Firefox Nightly 93.0a1 (2021-08-19) (64-bit)
  • Firefox 91.0.1 (64-bit)

Unaffected version:

  • Firefox ESR 78.13.0esr (64-bit)

Steps to reproduce:

  1. Visit attached formvalidity-windowopen.html
  2. Click "Start Spoof validationMessage" button
  3. validationMessage will appear on Twitter website
  4. If you're logged in, then press Enter to like the tweet
Flags: sec-bounty?

Related to bug 1366818 a little, but the fixes probably need to be orthogonal.

Neil, I suspect this is because we used the browser.popupAnchor before, and don't anymore, so now tabswitches that destroy the anchor's frame no longer cause the popup to hide. Is that plausible?

I suspect we should make the form validation popup tabspecific, or perhaps even locationspecific, once bug 1724668 lands...

Type: task → defect
Component: Security → XUL Widgets
Flags: needinfo?(enndeakin)
Keywords: regression
OS: Unspecified → All
Product: Firefox → Toolkit
Hardware: Unspecified → Desktop
See Also: → CVE-2021-38508
Version: unspecified → Firefox 91

Oh, and thank you, Irvan, for the detailed analysis of what versions are affected and finding a regression window! Super helpful. If you don't mind sharing, I'm curious how you found this particular issue, and particularly if it was somewhat chance, and then you realized later it didn't work in esr, or if you looked at commits or other patterns. :-)

(In reply to :Gijs (out; back Aug 31st; he/him) from comment #3)

Oh, and thank you, Irvan, for the detailed analysis of what versions are affected and finding a regression window! Super helpful.

Thanks Gijs for the update, by using mozregression I can easily identify the regression window, it's a very useful tool to find out what causing the bugs.

If you don't mind sharing, I'm curious how you found this particular issue, and particularly if it was somewhat chance, and then you realized later it didn't work in esr, or if you looked at commits or other patterns. :-)

I was playing with HTML5 API especially HTMLFormElement.reportValidity() then after the XUL dialog appears, I then press Ctrl + W to close the tab, surprisingly the XUL dialog still appear on current tab (reported as bug 1713259).

Then after a few month I revisited the testcase, I recently realize I able to switch tab with Alt+2, after switch tab with the keyboard shortcut surprisingly the XUL dialog is also still appear on target website (reported as this bug) =).

Neil, I suspect this is because we used the browser.popupAnchor before, and don't anymore, so now tabswitches that destroy the anchor's frame no longer cause the popup to hide. Is that plausible?

The popup should be anchored by rectangle passed from the child process. There isn't any mechanism that hides it in the popup code.

But it looks like there might be some browser-piece that does some of this, but I don't have any extra knowledge of that than you would.

Flags: needinfo?(enndeakin)

Possible dupe of bug 1718571? That blames blames a different regressor: bug 1680637

Flags: needinfo?(enndeakin)
Regressed by: 1684792
See Also: → CVE-2021-38509
Has Regression Range: --- → yes

[Tracking Requested - why for this release]:

You don't get a lot of control to pull off spoofing, but the result is so unusual that it's easy to believe at least some people being suckered in. There's no interaction there so the harm is fully down to what your text can convince someone to do.

(In reply to Daniel Veditz [:dveditz] from comment #6)

Possible dupe of bug 1718571? That blames blames a different regressor: bug 1680637

I can't see that bug.

Flags: needinfo?(enndeakin)

I've CC'ed you.

(In reply to Neil Deakin from comment #5)

Neil, I suspect this is because we used the browser.popupAnchor before, and don't anymore, so now tabswitches that destroy the anchor's frame no longer cause the popup to hide. Is that plausible?

The popup should be anchored by rectangle passed from the child process. There isn't any mechanism that hides it in the popup code.

Sure, but before bug 1684792 we anchored it to a specific DOM node and its frame, rather than a rect directly. When we anchor a popup to a DOM node's frame, doesn't destruction of the frame (because the node gets hidden) mean the popup gets hidden, like moving the frame means the popup moves? Like e.g. bug 1725151.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attached file WIP: Bug 1726621 - tests (obsolete) —

Depends on D124268

Blocks: 1731665

Comment on attachment 9239044 [details]
WIP: Bug 1726621 - tests

Revision D124269 was moved to bug 1731665. Setting attachment 9239044 [details] to obsolete.

Attachment #9239044 - Attachment is obsolete: true

Comment on attachment 9239043 [details]
Bug 1726621 - ensure form validation popup always hides on tabswitches, navigations, etc., r?NeilDeakin

Beta/Release Uplift Approval Request

  • User impact if declined: sec-moderate security issue
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 0
  • List of other uplifts needed: n/a
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Relatively straightforward changes to how the popup validation code works.
  • String changes made/needed: N/A
Attachment #9239043 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9239043 [details]
Bug 1726621 - ensure form validation popup always hides on tabswitches, navigations, etc., r?NeilDeakin

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-moderate issue early in the ESR lifetime
  • User impact if declined: see above
  • Fix Landed on Version: 94, uplift to 93
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Relatively straightforward change.

Note: ESR form doesn't have a "also needs uplift" field, but uplifting this to 91esr requires uplifting https://bugzilla.mozilla.org/show_bug.cgi?id=1724668 too. I think the same risk assessment applies to that patch, but it got marked wontfix for esr91...

  • String or UUID changes made by this patch: Nope
Attachment #9239043 - Flags: approval-mozilla-esr91?
Depends on: 1724668

Landed: https://hg.mozilla.org/integration/autoland/rev/724917f8288f0f3ed6d752886f5f806b10750d5b

Backed out for failing browser-chrome's toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js:

https://hg.mozilla.org/integration/autoland/rev/c0c56312270bf844146e2de5301fc200ae1e1735

Push which ran failed tasks: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=1fc975f877678b2dee224c9d729357c5fca3395d&selectedTaskRun=WlfLgjpjR62jQO0YPdKCNw.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=352154709&repo=autoland

[task 2021-09-20T21:29:52.083Z] 21:29:52     INFO - TEST-PASS | toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js | Triggering menuitem # 0 - 
[task 2021-09-20T21:29:52.084Z] 21:29:52     INFO - waiting for verifyConfirmationHint
[task 2021-09-20T21:29:52.084Z] 21:29:52     INFO - Buffered messages logged at 21:25:36
[task 2021-09-20T21:29:52.085Z] 21:29:52     INFO - Console message: [JavaScript Warning: "telemetry.state_file_read_errors - Unknown scalar."]
[task 2021-09-20T21:29:52.086Z] 21:29:52     INFO - Console message: [JavaScript Warning: "telemetry.generated_new_client_id - Unknown scalar."]
[task 2021-09-20T21:29:52.087Z] 21:29:52     INFO - Buffered messages logged at 21:26:51
[task 2021-09-20T21:29:52.089Z] 21:29:52     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
[task 2021-09-20T21:29:52.090Z] 21:29:52     INFO - Buffered messages logged at 21:27:36
[task 2021-09-20T21:29:52.093Z] 21:29:52     INFO - Console message: [JavaScript Error: "1632173256526	addons.xpi	ERROR	System addon update list error Error: got node name: html, expected: updates" {file: "resource://gre/modules/Log.jsm" line: 723}]
[task 2021-09-20T21:29:52.094Z] 21:29:52     INFO - append@resource://gre/modules/Log.jsm:723:12
[task 2021-09-20T21:29:52.094Z] 21:29:52     INFO - log@resource://gre/modules/Log.jsm:379:16
[task 2021-09-20T21:29:52.095Z] 21:29:52     INFO - error@resource://gre/modules/Log.jsm:387:10
[task 2021-09-20T21:29:52.095Z] 21:29:52     INFO - updateSystemAddons/res<@resource://gre/modules/addons/XPIInstall.jsm:4042:25
[task 2021-09-20T21:29:52.095Z] 21:29:52     INFO - 
[task 2021-09-20T21:29:52.096Z] 21:29:52     INFO - Buffered messages finished
[task 2021-09-20T21:29:52.097Z] 21:29:52     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js | Test timed out - 
Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #16)

Landed: https://hg.mozilla.org/integration/autoland/rev/724917f8288f0f3ed6d752886f5f806b10750d5b

Backed out for failing browser-chrome's toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js:

https://hg.mozilla.org/integration/autoland/rev/c0c56312270bf844146e2de5301fc200ae1e1735

Push which ran failed tasks: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=1fc975f877678b2dee224c9d729357c5fca3395d&selectedTaskRun=WlfLgjpjR62jQO0YPdKCNw.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=352154709&repo=autoland

[task 2021-09-20T21:29:52.083Z] 21:29:52     INFO - TEST-PASS | toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js | Triggering menuitem # 0 - 
[task 2021-09-20T21:29:52.084Z] 21:29:52     INFO - waiting for verifyConfirmationHint
[task 2021-09-20T21:29:52.084Z] 21:29:52     INFO - Buffered messages logged at 21:25:36
[task 2021-09-20T21:29:52.085Z] 21:29:52     INFO - Console message: [JavaScript Warning: "telemetry.state_file_read_errors - Unknown scalar."]
[task 2021-09-20T21:29:52.086Z] 21:29:52     INFO - Console message: [JavaScript Warning: "telemetry.generated_new_client_id - Unknown scalar."]
[task 2021-09-20T21:29:52.087Z] 21:29:52     INFO - Buffered messages logged at 21:26:51
[task 2021-09-20T21:29:52.089Z] 21:29:52     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
[task 2021-09-20T21:29:52.090Z] 21:29:52     INFO - Buffered messages logged at 21:27:36
[task 2021-09-20T21:29:52.093Z] 21:29:52     INFO - Console message: [JavaScript Error: "1632173256526	addons.xpi	ERROR	System addon update list error Error: got node name: html, expected: updates" {file: "resource://gre/modules/Log.jsm" line: 723}]
[task 2021-09-20T21:29:52.094Z] 21:29:52     INFO - append@resource://gre/modules/Log.jsm:723:12
[task 2021-09-20T21:29:52.094Z] 21:29:52     INFO - log@resource://gre/modules/Log.jsm:379:16
[task 2021-09-20T21:29:52.095Z] 21:29:52     INFO - error@resource://gre/modules/Log.jsm:387:10
[task 2021-09-20T21:29:52.095Z] 21:29:52     INFO - updateSystemAddons/res<@resource://gre/modules/addons/XPIInstall.jsm:4042:25
[task 2021-09-20T21:29:52.095Z] 21:29:52     INFO - 
[task 2021-09-20T21:29:52.096Z] 21:29:52     INFO - Buffered messages finished
[task 2021-09-20T21:29:52.097Z] 21:29:52     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js | Test timed out - 

Sigh, this test is racy and expects that it can wait for a confirmation hint even if it destroys the tab that created the form submit password saved doorhanger. It wins the race pre-patch, but loses post-patch because the code for tabspecific and locationspecific attributes became stricter about what it considers open/closed panels when switching tabs.

I'll fix the test and trypush to be safe, I guess, now that this has hit autoland anyway...

Flags: needinfo?(gijskruitbosch+bugs)
Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch

Comment on attachment 9239043 [details]
Bug 1726621 - ensure form validation popup always hides on tabswitches, navigations, etc., r?NeilDeakin

Approved for our last 93 beta, thanks.

Attachment #9239043 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Comment on attachment 9239043 [details]
Bug 1726621 - ensure form validation popup always hides on tabswitches, navigations, etc., r?NeilDeakin

Approved for 91.2esr.

Attachment #9239043 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
QA Whiteboard: [qa-triaged] → [qa-triaged][post-critsmash-triage]

Reproduced the issue reported in comment 0 using old Nightly build 93.0a1 (2021-08-19). Verified that using latest Nightly 94.0a1, Firefox 93.0b9 and latest ESR 91 build from treeherder across platforms (Windows 10, Ubuntu 18.04 and macOS 11.5) the validation message is no longer displayed when using the testcase.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged][post-critsmash-triage] → [post-critsmash-triage]
Flags: qe-verify+
Flags: sec-bounty? → sec-bounty+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-
Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv- → [reporter-external] [client-bounty-form] [verif?][adv-main93+]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main93+] → [reporter-external] [client-bounty-form] [verif?][adv-main93+][adv-esr91.2+]
Attached file advisory.txt
Alias: CVE-2021-38497
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: