Closed Bug 1570868 Opened 2 years ago Closed 2 months ago

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

Categories

(WebExtensions :: General, defect, P1)

All
Unspecified
defect

Tracking

(firefox88 fixed)

RESOLVED FIXED
88 Branch
Tracking Status
firefox88 --- fixed

People

(Reporter: sanjaygondaliya.infosec, Assigned: zombie)

Details

(Keywords: csectype-dos, hang, testcase, Whiteboard: [reporter-external] [client-bounty-form] )

Attachments

(5 files)

Attached image 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

Flags: sec-bounty?

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!

Flags: needinfo?(sanjaygondaliya.infosec)

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.

Type: task → defect
Keywords: crash, testcase
Summary: Firefox crash on windows and High CPU Usage on Mac and Ubuntu → Firefox crash on windows and High CPU Usage on Mac and Ubuntu (possibly requires extension)

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.

Group: firefox-core-security
Component: Security → Untriaged
Keywords: crashhang

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.

Flags: needinfo?(sanjaygondaliya.infosec)

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: Unspecified → All

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.

Component: Untriaged → General
Product: Firefox → WebExtensions
Version: unspecified → Trunk
Flags: needinfo?(jmathies)
Priority: -- → P3
Summary: Firefox crash on windows and High CPU Usage on Mac and Ubuntu (possibly requires extension) → Wappalyzer related content hang when interacting with Facebook
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?] [webext?]
Flags: needinfo?(mixedpuppy)

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

Flags: needinfo?(mixedpuppy) → needinfo?(fdiciocco)
Attached image 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.

Flags: needinfo?(fdiciocco)

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

Flags: needinfo?(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:

          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").

Flags: needinfo?(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.

Attached file 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)

to find someone who knows regex platform code to look at

Flags: needinfo?(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.

Flags: needinfo?(mixedpuppy) → needinfo?(jorendorff)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [webext?] → [reporter-external] [client-bounty-form] [verif?]

(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.

Flags: needinfo?(jmathies)

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

Flags: needinfo?(jorendorff) → needinfo?(sdetar)

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:

  1. 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.

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

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.

Flags: needinfo?(sdetar)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?] webext?
Whiteboard: [reporter-external] [client-bounty-form] [verif?] webext? → [reporter-external] [client-bounty-form]
Priority: P3 → P2
Flags: needinfo?(tomica)
Priority: P2 → P3
Whiteboard: [reporter-external] [client-bounty-form] → [reporter-external] [client-bounty-form] [verif?] webext?
Flags: needinfo?(tomica)
Priority: P3 → P2
Whiteboard: [reporter-external] [client-bounty-form] [verif?] webext? → [reporter-external] [client-bounty-form]
Flags: needinfo?(tomica)
Assignee: nobody → tomica
Flags: needinfo?(tomica)

Denial of service bugs are excluded from our bounty program

Flags: sec-bounty? → sec-bounty-
Keywords: csectype-dos

Did this bug assign CVE id?

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

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 security@mozilla.org -- they generally can't be answered by the developers working on the issue and are off-topic for the bug discussion.

Flags: needinfo?(dveditz)
Duplicate of this bug: 1693271

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

Assignee: tomica → nobody
Severity: normal → --
Priority: P2 → --
Summary: Wappalyzer related content hang when interacting with Facebook → Avoid browser hang triggered by extensions bindly passing URLs to `tabs.query` without escaping wildcards

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

Assignee: nobody → tomica
Status: NEW → ASSIGNED
Severity: -- → S1
Priority: -- → P1
Pushed by tjovanovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fdde8c6c8ddb
Ignore redundant wildcards in MatchGlob, avoid regex backtracking r=robwu
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
You need to log in before you can comment on or make changes to this bug.