1587867 - Pasting a lot of URLs into the URL bar crashes/hangs Firefox

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[miaomeow]

Attachment: firefox-dos.mkv

• Discovering the issue -
  This issue was discovered via I accidentally copy and pasted a full list of subdomains from another website into Firefox search bar, it crashed.
  The version I used is Firefox Quantum 69.0 64 bit, I am unsure whether it still works in 32 bit version.
  After verifying with my friend, I conclude that Windows and Linux both works.
  I had verify it with Chrome, Brave and Opera, they didn't crash but just show address not found.
  I also tested it with Nightly build and Firefox Beta, both of them crashed too.

• Steps to reproduce -
  Since I haven't found the root cause for it, I'll be using the website's subdomain as proof of concept.

1. Navigate to https://pastebin.com/raw/Sb6bbGQH
2. Copy all by using "Ctrl + A" and "Ctrl + C"
3. Open a new tab by using "Ctrl + T"
4. Paste in search bar by using "Ctrl + V"

• Expected results -
  Address not found

• Actual results -
  Firefox stopped working and crashed

• Crash reports -
  I tried checking my crash report as shown in https://developer.mozilla.org/en-US/docs/Mozilla/How_to_get_a_stacktrace_for_a_bug_report
  However it only shows InstallTime20190828152820 with the value of 1570725503 and an empty "events" directory.

• Closing -
  Attachment is a POC I made for the crash.
  Looking forward to hearing from you.

Comment 1

[Tyson Smith [:tsmith]]

This does case a hang but I was unable to reproduce the crash on Windows or Linux. Tested with a release build, a nightly build and an ASan build.

Comment 2

[Johann Hofmann [:johannh]]

I would go a step further and say this can not be considered a security issue. Doing weird stuff to your browser can break your browser and that specific instance could probably be fixed, but I fail to see the security aspect of it.

Comment 3

[miaomeow]

(In reply to Tyson Smith [:tsmith] from comment #1)

This does case a hang but I was unable to reproduce the crash on Windows or Linux. Tested with a release build, a nightly build and an ASan build.

I had tried and verify it with Windows and Linux myself, may I ask what are the system specs of your computer?

(In reply to Johann Hofmann [:johannh] from comment #2)

I would go a step further and say this can not be considered a security issue. Doing weird stuff to your browser can break your browser and that specific instance could probably be fixed, but I fail to see the security aspect of it.
Of course, a user wouldn't actually do this in the browser, and with JavaScript or a web page it's not hard to trick the user into doing it. As for the security issue, other than tricking users or crash their browser while they have any unsaved work, I do not think of any other possibilities yet.

However, this will cause unnecessary problems to the Mozilla team and the users, although it's not a security bug I do believe it should be patched so in the future it doesn't create any problems.

Comment 4

[rgfulw]

Hi there,

I have created a PoC available @ https://firefoxcrash.000webhostapp.com/.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a_
remember to include '_' in URL
This site was created by me for this specific purpose, nothing malicious.

The site contains the following:
"
If you are using Mozilla Firefox ver <69.0.1
You should not see this page after removing the / between .com'/'.a.a.a.a...
Removing // from http:'//'firefoxcrash may also be required.

Otherwise, this issue has been patched or you are not using the correct browser.
"

Simple plain text explaining how to exploit this crash, all the user needs to do is remove all '/' from the address bar.
Tested on latest Firefox 69.0.1 and multiple older versions.

This seems to only affect Mozilla Firefox Linux & Windows, yet to try out MacOS. Android is not affected.

For this exploit to work:

• Requires the URL to include .a-z x26 (I don’t know why, it just works) the more apended, the higher chances of crashing remote browser.
• Requires lowercase char before _ (text can also be appended to '_')
• Requires no '/' char in URL header.

I kindly request 'triage owner' please look at these steps in order to reproduce this issue.

Comment 5

[Drew Willcoxon :adw]

Thanks for the test case. I can reproduce it. I'll try to see what's going on.

Comment 6

[Drew Willcoxon :adw]

We hang/crash here: https://searchfox.org/mozilla-central/rev/696cf3b52a9aa04aa5013008a8b448904a298356/toolkit/components/places/UnifiedComplete.jsm#532

That line ends up being:

/^([\[\]A-Z0-9.:-]+[\.:]){3,}[\[\]A-Z0-9.:-]+$/i.test("httpsfirefoxcrash.000webhostapp.com.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a_")

If you run that in the browser console by itself, Firefox hangs. I'm not sure why yet.

Comment 7

[Drew Willcoxon :adw]

The number of .a's is important. Up to a certain number, there's no hang. After that, there starts to be hang, small at first, and the amount of hang increases with more .a's. It doesn't seem to increase linearly though because it doesn't take many until it starts to seem hung forever.

So rather than being an infinite loop, we seem to have triggered some really bad performance in the regexp engine.

Comment 8

[rgfulw]

Yes, first I was thinking it was long strings in the address bar. But removing all underscores from the list reported by miaomeow loads fine.

'google.com' x25 does the exact same thing as '.a'. Only after x26 we start to encounter slowdown and crashes.

So the length of chars does not quite matter, perhaps the way Firefox handles subdomains? Its odd how this requires no other symbols in the address bar for successful slowdown and crash, if there is an additional symbol in the URL it loads fine.

One other thing, TOR browser is not affected by this issue. Yet to test on MacOS VM.

Comment 9

[rgfulw]

(In reply to Drew Willcoxon :adw from comment #6)

That line ends up being:

/^([\[\]A-Z0-9.:-]+[\.:]){3,}[\[\]A-Z0-9.:-]+$/i.test("httpsfirefoxcrash.000webhostapp.com.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a_")

If you run that in the browser console by itself, Firefox hangs. I'm not sure why yet.

I did test this out with latest stable build of Firefox on Windows 10, I can confirm this causes a pop-up window mentioning the tab is slow.
But did not notice any slowdown or crash, just slightly more CPU being used (~+8%).

https://imgur.com/a/lRAIXH8

Comment 10

[Drew Willcoxon :adw]

Attachment: Bug 1587867 - Replace regexp in looksLikeURL with other logic to avoid bad performance for long strings.

The regexp checks for four parts separated by three dots [1]. This patch replaces the regexp with splitting the string on "." and then checking a smaller regexp against each of the parts.

[1] It actually allows dots or colons, but the colon allowance isn't necessary because the function already would have returned true due to the ["/", "@", ":", "["].some(c => str.includes(c)) check earlier.

Comment 11

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7ced28aa04a680ee334ec12a547c09689d59d6b

Comment 12

[rgfulw]

(In reply to Drew Willcoxon :adw from comment #10)

Created attachment 9102690 [details]
Bug 1587867 - Replace regexp in looksLikeURL with other logic to avoid bad performance for long strings.

The regexp checks for four parts separated by three dots [1]. This patch replaces the regexp with splitting the string on "." and then checking a smaller regexp against each of the parts.

[1] It actually allows dots or colons, but the colon allowance isn't necessary because the function already would have returned true due to the ["/", "@", ":", "["].some(c => str.includes(c)) check earlier.

Thank you very much!

Q1: - Would this be eligible for a CVE entry?
Q2: - Are we able to publicly disclose as PoC when patched, with demo to YT & PacketStormSecurity?

Comment 13

[Drew Willcoxon :adw]

(In reply to rgfulw from comment #8)

But removing all underscores from the list reported by miaomeow loads fine.

Interesting. If I remove the underscore from the string in comment 6, no hang. It doesn't have to be an underscore, though. If I replace it with a "?", the hang still happens. There must be something about that regexp.

One other thing, TOR browser is not affected by this issue.

This line was added here, in 64: https://hg.mozilla.org/mozilla-central/rev/8da12a6048fba4e59a860386b075f4d9070f79bf I don't know what version of Firefox the Tor browser is based on, but maybe it's an older one.

(In reply to rgfulw from comment #9)

I did test this out with latest stable build of Firefox on Windows 10, I can confirm this causes a pop-up window mentioning the tab is slow.
But did not notice any slowdown or crash, just slightly more CPU being used (~+8%).

Hmm, that's not what I would expect and suggests there might be more to this than that regexp.

Would you be able to try a test build to see if the patch fixed the problem for you?

Linux: https://queue.taskcluster.net/v1/task/OErNr6WcSp2JZqUGHkdBKg/runs/0/artifacts/public/build/target.tar.bz2
Linux x64: https://queue.taskcluster.net/v1/task/NKpZ-02DRbK2V3G7esoNNA/runs/0/artifacts/public/build/target.tar.bz2

Still waiting on Windows builds, but they'll appear at the link in comment 11 when they're done.

Comment 14

[Drew Willcoxon :adw]

(In reply to rgfulw from comment #12)

Q1: - Would this be eligible for a CVE entry?
Q2: - Are we able to publicly disclose as PoC when patched, with demo to YT & PacketStormSecurity?

I'm not an expert on security matters. Dan, can you answer? But I agree with Johann (comment 2) that this isn't a security problem, fwiw.

Comment 15

[rgfulw]

(In reply to Drew Willcoxon :adw from comment #14)

I'm not an expert on security matters. Dan, can you answer? But I agree with Johann (comment 2) that this isn't a security problem, fwiw.

While studying, I was taught any bug/vulnerability affecting any of the CIA traits is considered a security issue.

This bug affects the Availability of the content on all tabs, freezing the entire browser and not just the single tab.
If there was any unsaved data on other tabs, the information would be lost.

Comment 16

[rgfulw]

(In reply to Drew Willcoxon :adw from comment #13)

Would you be able to try a test build to see if the patch fixed the problem for you?

Linux x64: https://queue.taskcluster.net/v1/task/NKpZ-02DRbK2V3G7esoNNA/runs/0/artifacts/public/build/target.tar.bz2

I can confirm this build is not affected by this issue.

Same issue with console https://imgur.com/a/cOISJfp
(In reply to Drew Willcoxon :adw from comment #6)

We hang/crash here: https://searchfox.org/mozilla-central/rev/696cf3b52a9aa04aa5013008a8b448904a298356/toolkit/components/places/UnifiedComplete.jsm#532

That line ends up being:

/^([\[\]A-Z0-9.:-]+[\.:]){3,}[\[\]A-Z0-9.:-]+$/i.test("httpsfirefoxcrash.000webhostapp.com.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a_")

If you run that in the browser console by itself, Firefox hangs. I'm not sure why yet.

But great job on the address bar fix.

Comment 17

[Drew Willcoxon :adw]

Thanks for your help!

Comment 18

[Daniel Veditz [:dveditz]]

(In reply to rgfulw from comment #12)

Q1: - Would this be eligible for a CVE entry?

According to our understanding of CVE assignment rules, client denial of service bugs do not get a CVE.

Q2: - Are we able to publicly disclose as PoC when patched, with demo to YT & PacketStormSecurity?

This was made a public bug in comment 2 so feel free any time.

While studying, I was taught any bug/vulnerability affecting any of the CIA traits is considered a security issue.

Technically true, but in practice otherwise-unexploitable crashes in client software (attacks on the "Availability" leg of the triad) is treated as a quality issue.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c0d54b69f2e
Change regexp in looksLikeURL to avoid bad performance for long strings. r=mak

Comment 20

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/8c0d54b69f2e

Comment 21

[Drew Willcoxon :adw]

Comment on attachment 9102690 [details]
Bug 1587867 - Replace regexp in looksLikeURL with other logic to avoid bad performance for long strings.

Beta/Release Uplift Approval Request

• User impact if declined: This is a performance fix it would be nice to have sooner rather than later. It really only applies to edge cases, though. I wouldn't expect many users to hit this.
• 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: Please see comment 4
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Please note that although this specific bug doesn't have a test for it, we have many existing urlbar tests that cover this code path. The patch only change a regular expression used in the urlbar.
• String changes made/needed:

Comment 22

[Cristian Baica [:cbaica], Release Desktop QA]

Reproduced the issue using Fx68.0a1.

The issue is verified fixed using Fx72.0a1 on Windows 10, Ubuntu 18.04 and macOS 10.15. The browser no longer hangs when pasting "httpsfirefoxcrash.000webhostapp.com.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a_" in the URL bar.

Waiting on uplift to beta to change status and flags accordingly.

Comment 23

[Pascal Chevrel:pascalc]

Comment on attachment 9102690 [details]
Bug 1587867 - Replace regexp in looksLikeURL with other logic to avoid bad performance for long strings.

P1 perf bug, evaluated as low risk and verified as fixed by QA in nightly, uplift approved for 71 beta 5, thanks.

Comment 24

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8960870ce871

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Can we add a test for this?

Comment 26

[Cristian Baica [:cbaica], Release Desktop QA]

The issue is verified fixed using Fx71.0b5 on Ubuntu 18.04, Windows 10 x64 and macOS 10.14. The browser no longer hands when pasting and searching the text mentioned in c#22.
@Ryan Should we add a manual test for this?

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Depends on the answer to comment 25. This feels like something we could have automated tests for.

Comment 28

[Marco Bonardo [:mak]]

This sounds like entering fuzzy testing matters; testing just this case would be trivial, but it wouldn't bring much benefit, we should rather fuzzy testing with random input. Unfortunately that is also a lot more complicate. That said, if Drew wants to add a test it's fine. I didn't consider it critical, because this is one of those cases you're unlikely to hit again, while you may hit a thousands others that fuzzy testing may discover while this test would ignore.

Comment 29

[Drew Willcoxon :adw]

I agree with Marco. It looks like bug 1495327 and this bug hit the same regexp, so given that we've had two bugs now hit it, it's understandable to want a test for it. But the type of strings that triggered the two bugs are different. It's possible (hopefully unlikely) that there's some other latent pathological string that would perform poorly under the fix in this bug. So we could write a test that covers the two existing bugs for this regexp but end up missing new cases. That's a poor argument against writing a regression test, of course, so if you feel strongly that we should write one, I can. But longer term, fuzzing would be the way to go, especially considering we have many regexps in the urlbar, not only this one, and I see I have email from someone on the fuzzing team.

Comment 30

[Daniel Veditz [:dveditz]]

Unfortunately denial-of-service bugs are not eligible for our bug bounty program.

Comment 31

[rgfulw]

I released publicly recently a while ago. Perhaps other researchers are looking into this?

YT https://youtu.be/KJkCVUIaUEI
GitHub https://github.com/SocialEngineeringNeo/Exploits/blob/master/Our%20Exploits/Mozilla/Firefox/AddrBarDoS_Report.txt
PacketStorm https://packetstormsecurity.com/files/155808/Mozilla-Firefox-Denial-Of-Service.html

Comment 32

[Drew Willcoxon :adw]

That might explain the other similar bugs filed recently.