1739934 - (CVE-2021-43544) social-engineered XSS on default search provider via javascript:alert(1) URL which is SENT from another app

Status: VERIFIED FIXED

(Fenix :: General, defect)

Description

[Irwan]

Attachment: Uxss

hello Firefox security team,
i think it's a uxss vulnerability triggering android Firefox users, when i try to execute javascript:alert(1) in Firefox it doesn't work. to trigger it I use the Google translate app and translate javascript:alert(document.domain)// then I share it with Firefox users then uxss will be triggered there.

Production steps:

1. enter the android app and translate javascript:alert(1) // then share it to Firefox android
2. successfully shared Firefox will point to google, with a javascript:alert(1) search
3. just press once search on Firefox web then enter then uxss will be triggered.

scenario :
I think not everyone understands this problem, maybe when the javascript that is shared from the translate apps to Firefox users, ordinary victims will use the search feature of Firefox and accidentally press enter because there is no back button. with this xss will be triggered

PoC videos :
https://drive.google.com/file/d/1dEXjgypdjKb_eZNIW3Pf7WIY5Cjkmqq7/view?usp=sharing

Impact :
Uxss via javascript shared by translate apps to Firefox android users

Comment 1

[Irwan]

because I didn't understand initially reporting a bug on your system please understand, I have reported it also via ticket https://github.com/mozilla-mobile/fenix/issues/22356

Comment 2

[Irwan]

Hello ?

Comment 3

[Arturo Mejia [:amejia]]

Hi Daniel,
could we can get a sec level on this bug?
Thanks in advance!

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

2. successfully shared Firefox will point to google, with a javascript:alert(1) search
3. just press once search on Firefox web then enter then uxss will be triggered.

What happens is:

• Firefox receives the text "javascript:alert(1)" (via SEND intent, not VIEW intent like URI loads).
• We search for an URI in that text.
• We do not find any URI (I'm curious why though, it's not like we filter here explicitly)
• As fallback we construct a search URL for the text and load that
• When clicking into the URL bar we add the search text instead of the URI to it (if applicable) - to allow the user to quickly modify their search terms
• In this case the search terms are "javascript:alert(1)", which we identify as not being search terms and instead load the URI in Gecko

Not sure how an actual attack would look like? However there's definitely a mismatch between the URI/Search detection in the Intent handler and in the toolbar. However to some degree that may be intentional: I don't think we ever want to accept javascript: URIs via Intent, but we may want to allow the user to enter them manually.

Comment 6

[Irwan]

can you produce it if not please let me know I will give more detail POC..
I guess you can emulate the fix on Opera browser, they filter the javascript into their search field. Anchovy

Comment 7

[Daniel Veditz [:dveditz]]

Of course Google Translate isn't malicious, but it demonstrates that some other app could do this intentionally. In the example the external app doesn't get to target the UXSS at a particular domain, but it could probably load enough code to do interesting things depending on what popular site it finds itself on. There might be a way for another app to get us to open a specific target URL first, then send the javascript: url from the background?

I don't think we ever want to accept javascript: URIs via Intent

We don't ever want to accept javascript: URLs from anywhere for any reason. Maybe bookmarklets as a special exception, but those are so niche that the risks of allowing it from too many places isn't worth the risk.

but we may want to allow the user to enter them manually.

maybe. We sort of don't sometimes though. I guess we block pastes, but typing works

Comment 8

[Irwan]

hi is there any update there?

Comment 9

[Arturo Mejia [:amejia]]

Hi Irwan, sorry for the delay. We are working on a possible solution.
We are going to update the bug as soon a we have it concrete.

Comment 10

[Irwan]

Thanks for your reply, no problem, can you change the status to assign (trigaed) for this problem now?

Comment 11

[Arturo Mejia [:amejia]]

Attachment: AC.patch

This is a draft just to show the solution, I will put a final patch (including tests) after confirming this is the right approach.

Comment 12

[Arturo Mejia [:amejia]]

Attachment: Fenix.patch

Comment 13

[Arturo Mejia [:amejia]]

To summarize the patches.

We are blocking any JavaScript URLs by default, and only allowing them for bookmarks.

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 9250811 [details]
AC.patch

Unfortunately Bugzilla doesn't show the "review"/Splinter option here?

The patch looks good.

nit: I think we should move the getFlags() method from EngineSession (concept) to GeckoEngineSession, maybe as an extension method. Stripping this specific flag before passing it along is a Gecko-specific thing we do, so let's do it only internally in GeckoEngineSession (We could also try naming it in a way that it is more clear that we are filtering the flags down to Gecko supported flags.

Comment 15

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 9250815 [details]
Fenix.patch

LGTM. Only question I have is: We added this "recently saved" section to home recently. Is this covered too?

Comment 16

[Arturo Mejia [:amejia]]

(In reply to Sebastian Kaspari (:sebastian; :pocmo) from comment #14)

Comment on attachment 9250811 [details]
AC.patch

Unfortunately Bugzilla doesn't show the "review"/Splinter option here?

The patch looks good.

nit: I think we should move the getFlags() method from EngineSession (concept) to GeckoEngineSession, maybe as an extension method. Stripping this specific flag before passing it along is a Gecko-specific thing we do, so let's do it only internally in GeckoEngineSession (We could also try naming it in a way that it is more clear that we are filtering the flags down to Gecko supported flags.

Thanks for the review.
Got it, no problem.

Comment 17

[Arturo Mejia [:amejia]]

(In reply to Sebastian Kaspari (:sebastian; :pocmo) from comment #15)

Comment on attachment 9250815 [details]
Fenix.patch

LGTM. Only question I have is: We added this "recently saved" section to home recently. Is this covered too?

Yes, we are adding the flag on DefaultRecentBookmarksController to cover that :).

Comment 18

[Christian Sadilek [:csadilek]]

Comment on attachment 9250811 [details]
AC.patch

On the A-C patch, I think we'll need to update our BookmarksStorageSuggestionProvider to pass thew new flag along (in onSuggestionClicked). Users open bookmarklets through the awesomebar as a workaround to bookmarks always being opened in new tabs otherwise. So since we're doing this to keep bookmarklets working, we need to make sure this keeps working as well.

We should add a comment to getFlags explaining why we're removing the flag before passing it to GV e.g. just to point out that this flag is A-C specific.

Approach looks good to me!

Comment 19

[Christian Sadilek [:csadilek]]

Comment on attachment 9250815 [details]
Fenix.patch

Fenix side looks good to me for the immediate fix. I think we will want to introduce a BookmarksUseCase class here in a follow-up and also address the problem that bookmarks are always opened in a new tab, but let's keep this separate.

Comment 20

[Arturo Mejia [:amejia]]

(In reply to Christian Sadilek [:csadilek] from comment #18)

Comment on attachment 9250811 [details]
AC.patch

On the A-C patch, I think we'll need to update our BookmarksStorageSuggestionProvider to pass thew new flag along (in onSuggestionClicked). Users open bookmarklets through the awesomebar as a workaround to bookmarks always being opened in new tabs otherwise. So since we're doing this to keep bookmarklets working, we need to make sure this keeps working as well.

We should add a comment to getFlags explaining why we're removing the flag before passing it to GV e.g. just to point out that this flag is A-C specific.

Approach looks good to me!

Perfect!
I will update the patches the suggestions.
Thanks for the review.

Comment 21

[Arturo Mejia [:amejia]]

Attachment: AC.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not very easy

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
We are not going to include tests, until the fix lands on the release version.

Which older supported branches are affected by this flaw?
Firefox 95 and below

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Yes, we will backport.

How likely is this patch to cause regressions; how much testing does it need?
No likely.

Comment 22

[Arturo Mejia [:amejia]]

Attachment: Fenix.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not very easy

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
We are not going to include tests, until the fix lands on the release version.

Which older supported branches are affected by this flaw?
Firefox 95 and below

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Yes, we will backport.

How likely is this patch to cause regressions; how much testing does it need?
No likely.

Comment 23

[Arturo Mejia [:amejia]]

Attachment: AC_final.patch

This replaced https://bugzilla.mozilla.org/show_bug.cgi?id=1739934#c21 as shows the Splinter Review.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not very easy

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
We are not going to include tests, until the fix lands on the release version.

Which older supported branches are affected by this flaw?
Firefox 95 and below

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Yes, we will backport.

How likely is this patch to cause regressions; how much testing does it need?
No likely.

Comment 24

[Tom Ritter [:tjr] (OOTO until mid-August)]

Comment on attachment 9251049 [details] [diff] [review]
AC_final.patch

I think this going to be fine to land, but I'm clearing sec-approval until we have r+'s just in case it takes some cycles.

Comment 25

[Christian Sadilek [:csadilek]]

Comment on attachment 9251049 [details] [diff] [review]
AC_final.patch

Review of attachment 9251049 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Just a nit.

::: components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ +183,5 @@
>  
> +    private fun shouldLoadJSSchemes(
> +        scheme: String?,
> +        flags: LoadUrlFlags
> +    ) = scheme?.startsWith(JS_SCHEME) == true && flags.contains(ALLOW_JAVASCRIPT_URL)

nit: The scheme is already normalized here, so we could really check for equality instead of `startsWith`. That's also what happens when checking BLOCKED_SCHEMES.contains.

Comment 26

[Christian Sadilek [:csadilek]]

Comment on attachment 9251047 [details] [diff] [review]
Fenix.patch

Review of attachment 9251047 [details] [diff] [review]:
-----------------------------------------------------------------

::: app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt
@@ +152,5 @@
>          activity.openToBrowserAndLoad(
>              searchTermOrURL = url,
>              newTab = fragmentStore.state.tabId == null,
> +            from = BrowserDirection.FromSearchDialog,
> +            flags = flags

nit: Why this change? That's the default here anyway, no?

::: app/src/main/java/org/mozilla/fenix/search/SearchDialogInteractor.kt
@@ +30,5 @@
>          searchController.handleTextChanged(text)
>      }
>  
> +    override fun onUrlTapped(url: String, flags: LoadUrlFlags) {
> +        searchController.handleUrlTapped(url, flags)

nit: same q: above. why change this?

Comment 27

[Arturo Mejia [:amejia]]

(In reply to Christian Sadilek [:csadilek] from comment #26)

Comment on attachment 9251047 [details] [diff] [review]
Fenix.patch

Review of attachment 9251047 [details] [diff] [review]:

::: app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt
@@ +152,5 @@

     activity.openToBrowserAndLoad(
         searchTermOrURL = url,
         newTab = fragmentStore.state.tabId == null,

•        from = BrowserDirection.FromSearchDialog,
  

•        flags = flags
  

nit: Why this change? That's the default here anyway, no?

::: app/src/main/java/org/mozilla/fenix/search/SearchDialogInteractor.kt
@@ +30,5 @@

     searchController.handleTextChanged(text)
 }

• override fun onUrlTapped(url: String, flags: LoadUrlFlags) {

•    searchController.handleUrlTapped(url, flags)
  

nit: same q: above. why change this?

We need both as we added the flags AC on BookmarksStorageSuggestionProvider, then we are receiving the flags on AwesomeBarView.kt line 58, but never passing then to the interactor (that at the end delategates to the load use-case), without interactor.onUrlTapped(url, flags) this line the load use-case never get the flags from BookmarksStorageSuggestionProvider, for more information see AwesomeBarInteractor line 20.

The issue same can be say about additionalHeaders this is separate issue AwesomeBarView.kt line 58.

Comment 28

[Arturo Mejia [:amejia]]

Additionally to clarify the usages above, I filed this follow-up ticket https://github.com/mozilla-mobile/fenix/issues/22494 help to understand the reason why we need the extra code.

Comment 29

[Arturo Mejia [:amejia]]

Comment on attachment 9251049 [details] [diff] [review]
AC_final.patch

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not very easy
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: Firefox 95 and below
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: No likely

Comment 30

[Irwan]

Hi a little Addition, I see Uxss can be exploited further, as I explained above many ways to send the javascript via google translate or Name the application, even create bookmarks, with the javascript payload and then share it with other Firefox users with the same production steps.Thank you

Comment 31

[Tom Ritter [:tjr] (OOTO until mid-August)]

We should break the tests out into a separate patch that we land later (we typically land one release later.)

Comment 32

[Arturo Mejia [:amejia]]

(In reply to Irwan from comment #30)

Hi a little Addition, I see Uxss can be exploited further, as I explained above many ways to send the javascript via google translate or Name the application, even create bookmarks, with the javascript payload and then share it with other Firefox users with the same production steps.Thank you

Thanks Irwan!

Comment 33

[Arturo Mejia [:amejia]]

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #31)

We should break the tests out into a separate patch that we land later (we typically land one release later.)

Thanks Tom, yes this is the approach that we are going to take :)

Comment 34

[Tom Ritter [:tjr] (OOTO until mid-August)]

Okay; I can give sec-approval once that's done.

Comment 35

[Arturo Mejia [:amejia]]

No problem, I'll update the patches removing the tests.

Comment 36

[Arturo Mejia [:amejia]]

Attachment: AC.patch

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not very easy
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: Firefox 95 and below
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: No likely

Comment 37

[Arturo Mejia [:amejia]]

Attachment: Fenix.patch

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not very easy
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: Firefox 95 and below
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: No likely

Comment 38

[Tom Ritter [:tjr] (OOTO until mid-August)]

Comment on attachment 9252172 [details] [diff] [review]
Fenix.patch

Thanks! Approved to land.

Comment 39

[Tom Ritter [:tjr] (OOTO until mid-August)]

Comment on attachment 9252170 [details] [diff] [review]
AC.patch

Approved to land

Comment 40

[Arturo Mejia [:amejia]]

We landed the patch on Fenix (main), it will be available in the next nightly update. We are going to wait until early next week to uplift the patch to Beta 95 to give the patch some time on nightly to early discover any possible issues .

Comment 41

[Irwan]

I hope this problem can be solved before the weekend arrives :D

Comment 42

[Arturo Mejia [:amejia]]

Yesterday, we uplifted the patch to the beta branch 95 as we didn't found any issues on nightly, the patch should be available in the next beta update.

Comment 43

[Irwan]

thanks for your reply, then when can i hear about the bounty of this bug?

Comment 44

[Tom Ritter [:tjr] (OOTO until mid-August)]

I'm marking it fixed which will flag it for consideration for a bounty at our next committee meeting.

Comment 45

[Daniel Veditz [:dveditz]]

We triaged this too quickly initially and over-rated it. This is not a "Universal" XSS, it can only target your search provider. Our default in most of the world, Google, is a pretty interesting target if the user happens to have a Google login (lots of tied services). Less so if the user has customized their search. It also requires user interaction on an obvious javascript: url, after seeing the search results. sec-moderate is more appropriate.

Comment 46

[Irwan]

Well you are right the attacker can only target search sites like google, Bing, Yahoo etc. can also target other sites like Facebook but it is impossible for the victim to change the search engine They became Facebook.

Comment 47

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

(In reply to Arturo Mejia from comment #42)

Yesterday, we uplifted the patch to the beta branch 95 as we didn't found any issues on nightly, the patch should be available in the next beta update.

This ended up being the 95.1.0 RC build rather than a beta release.

Comment 48

[Irwan]

hi it's my first time to get a reward from Firefox, how do i withdraw the funds $1k

Comment 49

[Tom Ritter [:tjr] (OOTO until mid-August)]

I sent an email to you about 17 hours before you posted this comment; if you cannot find it, please contact me at [e-mail]

Comment 50

[Irwan]

thanks, I just saw because of the stack of messages, I will follow up on your email tom!

Comment 51

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 53

[Muhammad Zaid Ghifari]

Hi team,

it seems our browser and version are very different, why is this a duplicate?

Comment 54

[Daniel Veditz [:dveditz]]

Both your bug and this involves javascript: URLs sent via SEND. Can you reproduce your bug in a version that contains this fix?

Comment 55

[Irwan]

hi Daniel,
I see this is not a duplicate of what you said, because you confirmed the status of the fix in version 95.96. I think you missed the Firefox Beta fix. It is not like that?

Comment 56

[Muhammad Zaid Ghifari]

(In reply to Irwan from comment #55)

hi Daniel,
I see this is not a duplicate of what you said, because you confirmed the status of the fix in version 95.96. I think you missed the Firefox Beta fix. It is not like that?

Comment 57

[Daniel Veditz [:dveditz]]

I do not understand the question. Is the bug fixed in Firefox 95 (release) and Firefox 96 Beta ? Are you saying it is not fixed in Fireox 96 Beta?

Comment 58

[Muhammad Zaid Ghifari]

(In reply to Daniel Veditz [:dveditz] Out until January from comment #57)

I do not understand the question. Is the bug fixed in Firefox 95 (release) and Firefox 96 Beta ? Are you saying it is not fixed in Fireox 96 Beta?

no, the point is that when patching firefox 95, there was no patching on firefox beta 96, it happened when I first reported this vulnerability in my report on bug 1745842

Comment 59

[Muhammad Zaid Ghifari]

in short, he said i reported ahead of your patching and checking.

Comment 60

[Daniel Veditz [:dveditz]]

I don't know where he said that, but that contradicts the evidence in this bug and the way normal software development works. He is wrong.

• Nov 16: By comment 21 Arturo has a patch for the current development branch (96). In that comment he notes that 95 is also affected. He repeats that in comment 29 because "check if other versions are affected" is a standard part of software development.
• Nov 23: one of our Release Managers explicitly marks this as being tracked to be fixed in 95 and 96.
• Nov 25: fixed in "nightly" (the future Firefox 96)
• Nov 30: fixed in "beta" after testing in nightly (the future Firefox 95)
• Dec 1: Ryan notes that it missed the last beta release for 95 but was put into the 1st Release Candidate
• Dec 7: we release Firefox 95 and our advisories, and somewhere around here release 96 on the beta channel
• Dec 13: you file bug 1745842

All of that information is here, in this bug that you are reading. If your bug is fixed, it is fixed solely because of actions in this bug before you filed yours. Those actions are our standard actions and would have happened even if you had filed your bug a month earlier.

If your bug is not a duplicate then it should still be happening. We'll need a better PoC so we can distinguish the cases.

Comment 61

[Muhammad Zaid Ghifari]

Hi team,

I don't agree if it is said to be moderate, because this Xss can be executed on "Scan QR Barcode" which means this is a more serious problem "High",

where can i do a retest to prove that my analysis is correct??

Comment 62

[Muhammad Zaid Ghifari]

Attachment: UXss Barcode.jpeg

this is the PoC, please scan this barcode on the firefox beta URL bar and see the results.

you have to retest the vulnerable firefox browser before

Comment 63

[Kevin Brosnan [Ex-Mozilla]]

Your PoC in comment 62 is rather different than the bug filed here in comment 0. Your PoC comment 62 does not work on Firefox Beta 96 or Nightly 97 which means that it was fixed as part of bug 1705094. Which was fixed in nightly around November 8th. Which is over a month before you filed your bug. We strongly recommend researchers test their findings on Nightly versions of Firefox.

Comment 64

[Muhammad Zaid Ghifari]

(In reply to Kevin Brosnan [:kbrosnan] from comment #63)

Your PoC in comment 62 is rather different than the bug filed here in comment 0. Your PoC comment 62 does not work on Firefox Beta 96 or Nightly 97 which means that it was fixed as part of bug 1705094. Which was fixed in nightly around November 8th. Which is over a month before you filed your bug. We strongly recommend researchers test their findings on Nightly versions of Firefox.

I want you to retest the version that was pepatched last November 8, because I did not find access to download version 95, I think version 95 is vulnerable not at a moderate level but high, because it can be executed Firefox beta via barcode scan.

Comment 65

[Kevin Brosnan [Ex-Mozilla]]

The qr code scanner is a secondary method to get URLs and displays the URL that will be loaded. The user needs to confirm that the URL is what they want to load. It is currently the Christmas and New Years break and almost all the team is out of the office. The findings in comment 62 is not a critical security flaw such that I/Mozilla need to call 20+ people in from paid time off. Firefox 96 will contain the fix to your report from comment 62, it will be released on 2022-01-11.

Comment 66

[Irwan]

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #49)

I sent an email to you about 17 hours before you posted this comment; if you cannot find it, please contact me at [e-mail]

Hi tom, can you open your email, I want to discuss there?