1570868 - Avoid browser hang triggered by extensions bindly passing URLs to `tabs.query` without escaping wildcards

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Sanjay]

Attachment: Crash Dump.png

I found a specific patter which always leads to Crash or High CPU Usage

File and Vedio URL : https://drive.google.com/open?id=1L2pHSaAA5-5D6EdJ-YtrYWa0CFxJHSxS

Comment 1

[Johann Hofmann [:johannh]]

Thanks for the report. I don't think this is really actionable to us without the crash signature. Can you please go to about:crashes and submit and view your crash and add it to this bug?

This is also needed to determine whether this is a security issue.

Thanks!

Comment 2

[Daniel Veditz [:dveditz]]

The video shows this requiring the Wappalyzer extension plus the testcase. Could be a bug in the extension, or maybe the extension just happens to trigger the bug accidentally and it could happen in other situations.

Comment 3

[Daniel Veditz [:dveditz]]

Attachment: testcase from linked google drive

Comment 4

[Daniel Veditz [:dveditz]]

To save people having to piece this together from the video, here are the steps I see:

1. Install Wappalyzer https://addons.mozilla.org/en-US/firefox/addon/wappalyzer/
2. make sure you're logged-in to Facebook
3. open a page mid-way through the FB password-reset process, e.g. from the testcase
   https://www.facebook.com/recover/code/?em[0]=b***************1%40gmail.com&rm=send_email&hash=AUYRByaSGPpLRAFT

I have no idea if the testcase URL was found as part of a scam attempt, or if the reporter just happened to hit the bug when going through the reset flow. The video shows an unresponsive process on windows: a hang, not a crash. The .DBG output came from manually killing the process. We don't need to hide a hang bug.

Comment 5

[Sanjay]

I clarify that the issue is of firefox hang and i am not able to generate crash report so far.

The issue i discover during the Facebook penetration test and we use this "Wappalyzer" plugin to get information about website underlying technology and everything.

Initially i thought the issue is due to anything related to Facebook but after following the Mozzilla "https://support.mozilla.org/en-US/kb/firefox-crashes-troubleshoot-prevent-and-get-help" article i came to know that the issue is of Wappalyzer.

However, the firefox hang is persistent in major operating system that i have checked "Mac, Ubuntu and Windows".

The issue is dependent to Wappalyzer plugin but one can deeply analyze the vulnerability and create piece of code which could leads to DoS the firefox browser every-time.

Comment 6

[Florencia Di Ciocco, NI to vbalducci]

Hi,

Thanks for the details. I was able to reproduce the bug on Windows 10 on nightly 70.0a1 (2019-08-13) (64-bit). In my case the page didn't crash, just went unresponsive and I had to close the browser.

I haven't been able to choose a component. If you can, please do so.

Best regards, Flor.

Comment 7

[Sanjay]

Hi Team,

Does this bug false under bug bounty reward program or eligible to any CVE?

Comment 8

[Sanjay]

Hi Team,

Does this bug false under bug bounty reward program or eligible to any CVE?

Comment 9

[Sanjay]

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

To save people having to piece this together from the video, here are the steps I see:

1. Install Wappalyzer https://addons.mozilla.org/en-US/firefox/addon/wappalyzer/
2. make sure you're logged-in to Facebook
3. open a page mid-way through the FB password-reset process, e.g. from the testcase
   https://www.facebook.com/recover/code/?em[0]=b***************1%40gmail.com&rm=send_email&hash=AUYRByaSGPpLRAFT

I have no idea if the testcase URL was found as part of a scam attempt, or if the reporter just happened to hit the bug when going through the reset flow. The video shows an unresponsive process on windows: a hang, not a crash. The .DBG output came from manually killing the process. We don't need to hide a hang bug.

(In reply to Florencia Marina Di Ciocco from comment #6)

Hi,

Thanks for the details. I was able to reproduce the bug on Windows 10 on nightly 70.0a1 (2019-08-13) (64-bit). In my case the page didn't crash, just went unresponsive and I had to close the browser.

I haven't been able to choose a component. If you can, please do so.

Best regards, Flor.

Hi,

Does this bug false under bug bounty reward program or eligible to any CVE?

Comment 10

[Florencia Di Ciocco, NI to vbalducci]

Hi,

I've chosen a component. If you consider that there's another component that's more proper for this case you may change it.

Best regards, Flor.

Comment 11

[Sanjay]

(In reply to Florencia Marina Di Ciocco from comment #10)

Hi,

I've chosen a component. If you consider that there's another component that's more proper for this case you may change it.

Best regards, Flor.

Hi,

Does this bug false under bug bounty reward program or eligible to any CVE?

Comment 12

[Jim Mathies [:jimm]]

(In reply to Sanjay from comment #11)

(In reply to Florencia Marina Di Ciocco from comment #10)

Hi,

I've chosen a component. If you consider that there's another component that's more proper for this case you may change it.

Best regards, Flor.

Hi,

Does this bug false under bug bounty reward program or eligible to any CVE?

At this point no as it is a simple browser hang vs. an actual code exploit. Thank you though for reporting it, we'll take a look.

Comment 13

[Shane Caraveo (:mixedpuppy)]

I'm unable to reproduce this with the STR from comment 4 with either fx70 or fx71. Can we get a re-verification?

Comment 14

[Florencia Di Ciocco, NI to vbalducci]

Attachment: unresponsive.png

Hi,

For me, the STR from comment 4 works like a charm. I install the extension, log in on FB, go to the link and the page goes unresponsive. If I go to the page without installing the extension, FB doesn't crash. I'm working on Windows 10, Nightly 71.0a1 (2019-09-25).

Regards, Flor.

Comment 15

[Shane Caraveo (:mixedpuppy)]

Luca is going to give it a shot to see if he can reproduce the issue.

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

I have been finally able to reproduce the issue locally (took me a bit to get a Facebook account to test it) and dig into it a bit.

When Firefox is stuck, in a debug build Firefox keeps producing the following warnings:

[Parent 15845, Main Thread] WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 685
[Parent 15845, Main Thread] WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 685
[Parent 15845, Main Thread] WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 685

By interrupting the stuck Firefox instance I got the following C++ stacktrace:

#0  0x00007fffe51d8ef8 in XPCJSContext::InterruptCallback(JSContext*) () at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#1  0x00007fffe8893f37 in JSContext::handleInterrupt() () at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#2  0x00007fffe8a9621f in js::RegExpRunStatus js::irregexp::InterpretCode<unsigned char>(JSContext*, unsigned char const*, unsigned char const*, unsigned long, unsigned long, js::MatchPairs*, unsigned long*) () at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#3  0x00007fffe88878a7 in js::RegExpShared::execute(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*, unsigned long*) ()
    at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#4  0x00007fffe93dd023 in js::ExecuteRegExpLegacy(JSContext*, js::RegExpStatics*, JS::Handle<js::RegExpObject*>, JS::Handle<JSLinearString*>, unsigned long*, bool, JS::MutableHandle<JS::Value>) () at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#5  0x00007fffe88899c7 in JS::ExecuteRegExpNoStatics(JSContext*, JS::Handle<JSObject*>, char16_t const*, unsigned long, unsigned long*, bool, JS::MutableHandle<JS::Value>) ()
    at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#6  0x00007fffe8251f61 in mozilla::extensions::MatchGlob::Matches(nsTSubstring<char16_t> const&) const () at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#7  0x00007fffe8251d83 in mozilla::extensions::MatchPattern::Matches(mozilla::extensions::URLInfo const&, bool) const ()
    at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#8  0x00007fffe8252d01 in mozilla::extensions::MatchPatternSet::Matches(mozilla::extensions::URLInfo const&, bool) const ()
    at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#9  0x00007fffe5c259c6 in mozilla::dom::MatchPatternSet_Binding::matches(JSContext*, JS::Handle<JSObject*>, mozilla::extensions::MatchPatternSet*, JSJitMethodCallArgs const&) ()
    at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so
#10 0x00007fffe65fab33 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) () at /zfs-ubik/mozlab/shared/mozilla-central/objdir-frontend-debug/dist/bin/libxul.so

And the following JS stacktrace:

(gdb) call (void*)DumpJSStack()
0 matches(queryInfo = [object Object]) ["chrome://extensions/content/parent/ext-tabs-base.js":606:40]
    this = [object Object]
1 query() ["chrome://extensions/content/parent/ext-tabs-base.js":1989:37]
2 InterpretGeneratorResume(gen = [object Generator], val = undefined, kind = "next") ["self-hosted":1300:7]
3 next() ["self-hosted":1255:8]
    this = [object Generator]
4 from(items = [object Generator], mapfn = [function]) ["self-hosted":598:8]
    this = function Array() {
    [native code]
}
5 query(queryInfo = [object Object]) ["chrome://browser/content/parent/ext-tabs.js":957:23]
    this = [object Object]
6 query([object Object]) ["self-hosted":1019:16]
7 call/result</<() ["resource://gre/modules/ExtensionParent.jsm":1156:67]
8 withPendingBrowser(browser = null, callable = [function]) ["resource://gre/modules/ExtensionParent.jsm":767:25]
    this = [object Object]
9 call/result<() ["resource://gre/modules/ExtensionParent.jsm":1156:23]
10 callAndLog(context = [object Object], data = [object Object], callable = [function]) ["resource://gre/modules/ExtensionParent.jsm":1115:13]
    this = [object Object]
11 call() ["resource://gre/modules/ExtensionParent.jsm":1155:24]
12 AsyncFunctionNext(val = [function]) ["self-hosted":839:4]
    this = [object AsyncFunctionGenerator]

And so at a first glance it seems to me that the Firefox main process is getting stuck because:

• wappalyzer is calling browser.tabs.query with the url mentioned in Comment 4 (https://www.facebook.com/recover/code/?em[0]=b***************1%40gmail.com&rm=send_email&hash=AUYRByaSGPpLRAFT)

• the url is internally converted into a MatchPatternSet (by the query method defined in toolkit/components/extensions/child/ext-tabs.js):

          if (queryInfo.url !== null) {
            queryInfo.url = new MatchPatternSet([].concat(queryInfo.url), {
              restrictSchemes: false,
            });
          }

• then we try to find the tabs that match the pattern set by going through all the tabs and matching their url with the one in the MatchPatternSet
• and Firefox main process then gets stuck when it is trying to match the url from the tab that actually have the long url from the MatchPatternSet

Based on the C++ stack trace my guess is that testing the MatchPatternSet on that same url is taking too long (probably because of all the '*' chars in the pattern), Firefox would like to show the alert that tells the user that some JS code is taking too much time, but it is never able to actually show any dialog because we are unable to find a windows for it (as the warning, logged from XPCJSContext::InterruptCallback, that Firefox keeps printing to the console also suggests: "WARNING: No active window: file /builds/worker/workspace/build/src/js/xpconnect/src/XPCJSContext.cpp, line 685").

Comment 17

[Luca Greco [:rpl] [:luca] [:lgreco]]

For me the following "reduced test case" triggers the hang when executed in the BrowserConsole (and as a plus it doesn't need you to be logged on Facebook :-P):

let mps = new MatchPatternSet(["https://www.facebook.com/recover/code/?em[0]=b***************1%40gmail.com&rm=send_email&hash=AUYRByaSGPpLRAFT"])
mps.matches("https://www.facebook.com/recover/code/?em[0]=b***************1%40gmail.com&rm=send_email&hash=AUYRByaSGPpLRAFT")

The main difference seems that in this case the expected "A script on this page may be busy, or it may have stopped responding" dialog is being triggered and you can still recover the Firefox instance from its hang status.

Comment 18

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: test-bug-1570868.xpi

I just noticed that yesterday I forgot to also attach the minimal test extension which can reproduce the same kind of hang (and stack trace) using a similar example.com url instead of facebook.com (and so it doesn't need you to be logged in on Facebook).

Steps to reproduce:

• install the test extension from about:debugging
• click the browser action menu (which creates two tabs and at that point Firefox gets stuck on the tabs.query API, called from inside a webRequest.onCompleted listener, similarly to what wappalyzer does internally)

Comment 19

[Shane Caraveo (:mixedpuppy)]

to find someone who knows regex platform code to look at

Comment 20

[Shane Caraveo (:mixedpuppy)]

Jason, could you help find someone who could look into this? The particular string running through regex is causing a hang probably in ExecuteRegExpLegacy. Comment 17 is an easy repro.

Comment 21

[Sanjay]

Hi,

Does this bug false under bug bounty reward program or eligible to any CVE?

Comment 22

[Jim Mathies [:jimm]]

(In reply to Sanjay from comment #21)

Hi,

Does this bug false under bug bounty reward program or eligible to any CVE?

Don't think so, it's a either a bug or poor behavior in this extension.

Comment 23

[Shane Caraveo (:mixedpuppy)]

Steven, does this look like it belongs in js engine? See comment 16 and comment 17.

Comment 24

[Jason Orendorff [:jorendorff]]

Comment 16 is great stuff.

Without any extensions, run this JS code: /.*.*.*.*q/.exec("x".repeat(1000)). This regexp takes exponential time to fail—the implementation is doing a bunch of unnecessary backtracking.

MatchGlob::Init is producing a regexp like that one.

Options for fixing this inside the regexp engine (== in the Core :: JavaScript Engine bugzilla component):

1. As a special case: .* outside of all groups in a RegExp could be optimized to rule out backtracking, as long as the RegExp doesn't use backreferences. Not a priority for us.

2. By adding a completely different implementation (a finite state automaton, like the Rust regex crate) for patterns like this that can be matched without backtracking. Also not a priority.

Options for fixing this outside the regexp engine:

3. Completely rewrite MatchGlob to avoid using regular expressions. This would be easier than #1 or #2, but still a pain and there are probably better things to do.

4. Just ignore redundant * wildcards in MatchGlob::Init. Easy and would fix the particular bug here.

Comment 25

[Jason Orendorff [:jorendorff]]

Specifically, #3 means: Split the pattern on * and just search for each piece in turn. Backtracking across the * wildcards is never needed. You do need to write code to match a piece, which may contain ? wildcards if aAllowQuestion is true.

Comment 26

[Daniel Veditz [:dveditz]]

Denial of service bugs are excluded from our bounty program

Comment 27

[Sanjay]

Did this bug assign CVE id?

How to register in this list : https://cve.mitre.org?

Comment 28

[Sanjay]

Did this bug assign CVE id?

How to register in this list : https://cve.mitre.org?

Comment 29

[Daniel Veditz [:dveditz]]

We do not issue CVEs for client denial of service bugs. There is no need to coordinate this kind of bug fix with other vendors. If you have further questions about the bounty program or CVEs please mail [e-mail] -- they generally can't be answered by the developers working on the issue and are off-topic for the bug discussion.

Comment 31

[Luca Greco [:rpl] [:luca] [:lgreco]]

Resetting assignee, priority and severity to get this into the next triage meeting and re-triaging it.

Comment 32

[:Gijs (he/him)]

:kmag suggested in the dupe that we could switch to the rust regex engine if we wanted.

Comment 33

[Tomislav Jovanovic :zombie]

Attachment: Bug 1570868 - Ignore redundant wildcards in MatchGlob, avoid regex backtracking

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fdde8c6c8ddb
Ignore redundant wildcards in MatchGlob, avoid regex backtracking r=robwu

Comment 35

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/fdde8c6c8ddb