Closed Bug 1435319 (CVE-2018-12381) Opened 6 years ago Closed 6 years ago

Dragging an email from Outlook into a Firefox tab redirects to malicious site

Categories

(Core :: DOM: Navigation, defect)

57 Branch
Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 62+ fixed
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 + fixed

People

(Reporter: jana.squires, Assigned: Gijs)

References

Details

(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main62+][adv-esr60.2+])

User Story

There are two problems here:

1) dropping an email from the Outlook application on Windows gets interpreted as a URL of the outlook mail columns

2) fromsubjectreceivedsizecategories.com is a malicious site taking advantage of this and needs to get blocked.

Getting SafeBrowsing to block the site would mitigate this particular instance, but we're not having much success there. Besides there might be other apps (or a localized Outlook?) that result in different text that gets Firefox to open a different domain that bad people will then domain-squat on in a similar way.

Attachments

(6 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36

Steps to reproduce:

1. Open Outlook 2016 on a Windows device
2. Drag an email from Outlook, and drop it in a Firefox tab



Actual results:

1. Firefox then redirects to http://www.fromsubjectreceivedsizecategories.com/
2. That site in turns redirects to various malicious sites - an example is attached
3. The behaviour continues despite disabling the inline auto-complete option.  See http://kb.mozillazine.org/Inline_autocomplete

Note that the first site is a combination of the column headers in Outlook.  

Also note that I'm unable to reproduce this behaviour with Chrome or IE on Windows, or with Chrome, Firefox and Safari on Mac OSX.

Tested with only Firefox version 57.0.4


Expected results:

Firefox should not auto-complete the drag and drop from Outlook.
Can't open your image, there are errors in it (at least on my system -- maybe it's a variant .jpg format?)

The autocomplete option (if it still works -- that's an old article) would only apply to typing in a URL. Drag and drop is a different mechanism. There's no autocomplete, you're dropping the actual content. If the content you drop is a link to a web page we'll open that page. That page can then do whatever a web page can do that you've browsed to on the web or opened by clicking on a link elsewhere. Redirecting is one of those things.

We can report malicious sites to the SafeBrowsing service, so they get blocked. Otherwise I'm not sure what else here is a Firefox security bug.
Flags: needinfo?(jana.squires)
Hi Daniel,

Thanks for your feedback.

To clarify: This behaviour is reproduced with any email that's dragged from Outlook, and into a tab in Firefox.

The site that Firefox tries to navigate to is fromsubjectreceivedsizecategories[.]com/.   This is based on the default column headings within Outlook. 

I've been able to reproduce the behaviour today using Outlook 2016 (Windows) and Firefox 58.01 (Windows).  

I drag any email onto a page of a Firefox tab, an invalid page loads using then drag and drop mechanism, then navigation to the malicious site begins in a second tab.  Note it doesn't matter which email I use, the navigation to the aforementioned site is the same.

Are you able to test the behaviour as well?

Regards,
- Jana
Flags: needinfo?(jana.squires)
Attached image malicious site.png
Example of what loaded when I was initially testing the behaviour.
Comment on attachment 8953211 [details]
malicious site.png

Note that that the browser first attempts to connect to fromsubjectreceivedsizecategories[.]com and then may be redirected to another random site with a malicious payload - as per the attachment here.
Francois: do we have any mechanism to block sites when tracking protection is OFF (the default)? I believe Firefox is correctly interpreting the outlook content, but some scammer has obtained this domain that must be a default embedded in the format.

I've reported it to safebrowsing as well.
Status: UNCONFIRMED → NEW
Component: Untriaged → Security
Ever confirmed: true
Flags: needinfo?(francois)
Yes, we have the plugin blocklist: https://github.com/mozilla-services/shavar-plugin-blocklist/blob/master/mozplugin-block.txt

This list actually applies to any network loads, not just Flash loads.

It's controlled by browser.safebrowsing.blockedURIs.enabled and is default ON.
Flags: needinfo?(francois)
I reported this site to the safebrowsing people and it's still not blocked. Can we add it to our own list? This site does nothing good. At best you end up on Amazon with a referral link so these jerks make money. Using a Mac I quite often get stuck on a "tech support" scam site that locks up the browser using various eviltraps and I have to kill the child process to escape.
Flags: needinfo?(francois)
You're talking about http://www.fromsubjectreceivedsizecategories.com/?

As you point out, it's not flagged by Google: https://transparencyreport.google.com/safe-browsing/search?url=http:%2F%2Fwww.fromsubjectreceivedsizecategories.com%2F

In terms of adding it to our own blacklist, I'll need to check with Marshall. We have never had our own malware/phishing blacklist, so I need to make sure we can actually do this from a policy/legal point of view.
Flags: needinfo?(francois) → needinfo?(dveditz)
Someone has gamed Google's system: "No unsafe content found". There's nothing there except a navigation to various _other_ sites that are unsafe or deceptive, but the bad stuff isn't on that site so it gets a pass? The sites it navigated to kept changing so it'd be hard to report all those and get them all.
Flags: needinfo?(dveditz)
Thanks all for investigating this. 

Just want a reminder that during my tests I wasn't able to replicate this on Chrome or IE - only on Firefox. 

Admittedly I didn't test it with any other browsers.  However it does relate to the way Firefox handles drag and drops into tabs. Is there a way to disable that process in the configuration?
There are two problems here:

1) dropping an email from the Outlook application on Windows gets interpreted as a URL of the outlook mail columns. Why are we trying to interpret mail as anything? It's possible Outlook is tagging their drag content incorrectly, but other browsers seem to have made allowances for that if that's the case.

2) fromsubjectreceivedsizecategories.com is a malicious site taking advantage of this and needs to get blocked.

Getting SafeBrowsing to block the site would mitigate this particular instance, but we're not having much success there so we should look into #1. Besides there might be other apps (or a localized Outlook?) that result in different text that gets Firefox to open a different domain that bad people will then domain-squat on in a similar way.
Group: firefox-core-security → dom-core-security
User Story: (updated)
Component: Security → Drag and Drop
Flags: needinfo?(haftandilian)
OS: Unspecified → Windows
Product: Firefox → Core
unfortunately I don't have outlook 2016 right now.

can anyone perform the following operation to tell me what kind of data is dropped?
  1. open the attached file in Firefox 60.0.2, or newer, (or maybe on 58.0.1 if there's any different behavior)
     (the file listens to the drop event and shows dropped item's data)
  2. drag and drop an email from outlook 2016 to "Drag and drop items here." text in the page
  3. copy and paste the whole displayed text into a new text file
  4. upload the text file here
(In reply to François Marier [:francois] from comment #8)
> You're talking about http://www.fromsubjectreceivedsizecategories.com/?
> 
> As you point out, it's not flagged by Google:
> https://transparencyreport.google.com/safe-browsing/search?url=http:
> %2F%2Fwww.fromsubjectreceivedsizecategories.com%2F
> 
> In terms of adding it to our own blacklist, I'll need to check with
> Marshall. We have never had our own malware/phishing blacklist, so I need to
> make sure we can actually do this from a policy/legal point of view.

Did you ever check with Marshall on policy/legal aspects of adding this to the plugin blocklist? I nearly got caught out by this but luckily for me the corporate web content filtering saved me. Others aren't won't have such protections and I'm surprised issue has been hanging around for 4 months despite this being actively exploited. While it might not be the most elegant solution, adding the URL to the plugin blocklist may well mitigate the vulnerability for the vast majority of people.
(In reply to Tooru Fujisawa [:arai] from comment #13)
> Created attachment 8984726 [details]
> HTML file to show dropped items
> 
> unfortunately I don't have outlook 2016 right now.
> 
> can anyone perform the following operation to tell me what kind of data is
> dropped?
>   1. open the attached file in Firefox 60.0.2, or newer, (or maybe on 58.0.1
> if there's any different behavior)
>      (the file listens to the drop event and shows dropped item's data)
>   2. drag and drop an email from outlook 2016 to "Drag and drop items here."
> text in the page
>   3. copy and paste the whole displayed text into a new text file
>   4. upload the text file here

Richard or Jana, given your comments - are you able to perform these steps?
Flags: needinfo?(richard.j.stewart)
Flags: needinfo?(jana.squires)
Here's a screenshot of the result of the drag and drop test as requested
Flags: needinfo?(richard.j.stewart)
Thank you for checking!

So, apparently those words are separated with TAB character, which is somehow removed while fixing up.
(so, it's considered to be an URI inside ContentAreaDropListener.prototype._addLinksFromItem, and actually fixed up while opening it)

  let flags = Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS |
    Ci.nsIURIFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
  let info = Services.uriFixup.getFixupURIInfo("From\tSubject", flags);
  console.log(info.fixedURI.spec);

I'll look into the Services.uriFixup implementation.
(IMO, TAB should be treated in the same way as SPACE, which isn't get removed)


then, I misremembered about the current behavior:
  https://searchfox.org/mozilla-central/rev/d544b118e26422877108c42723b26e9bb4539721/dom/base/contentAreaDropListener.js#70-76
so, the issue is still happening on nightly.
Flags: needinfo?(jana.squires)
(In reply to Tooru Fujisawa [:arai] from comment #21)
> Thank you for checking!
> 
> So, apparently those words are separated with TAB character, which is
> somehow removed while fixing up.
> (so, it's considered to be an URI inside
> ContentAreaDropListener.prototype._addLinksFromItem, and actually fixed up
> while opening it)
> 
>   let flags = Ci.nsIURIFixup.FIXUP_FLAG_FIX_SCHEME_TYPOS |
>     Ci.nsIURIFixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP;
>   let info = Services.uriFixup.getFixupURIInfo("From\tSubject", flags);
>   console.log(info.fixedURI.spec);
> 
> I'll look into the Services.uriFixup implementation.
> (IMO, TAB should be treated in the same way as SPACE, which isn't get
> removed)

This is essentially a result of the URL specification, not fixup - `new URL("http://foo\tbar")` produces a URL object instead of null.

I guess when we prefix `http` ( inside https://dxr.mozilla.org/mozilla-central/rev/75a32b57132f8cba42779555662a057a0416a313/docshell/base/nsDefaultURIFixup.cpp#366 ) then we could in principle check if there are tab characters in the input and bail out if so. That seems hacky though, also because they could be in the query string or similar (rather than the hostname) where the current behavior is probably correct.

(FWIW, this probably wants moving back to a frontend component, or maybe Core :: Document Navigation if we think we should indeed fix this inside the URI fixup code, which in any case should probably be moved to toolkit at some point...)
Flags: needinfo?(arai.unmht)
(In reply to :Gijs (he/him) from comment #22)
> I guess when we prefix `http` ( inside
> https://dxr.mozilla.org/mozilla-central/rev/
> 75a32b57132f8cba42779555662a057a0416a313/docshell/base/nsDefaultURIFixup.
> cpp#366 ) then we could in principle check if there are tab characters in
> the input and bail out if so. That seems hacky though, also because they
> could be in the query string or similar (rather than the hostname) where the
> current behavior is probably correct.
> 
> (FWIW, this probably wants moving back to a frontend component, or maybe
> Core :: Document Navigation if we think we should indeed fix this inside the
> URI fixup code, which in any case should probably be moved to toolkit at
> some point...)

I think that we should probably address this in nsDefaultURIFixup, although it's not easy.
Some strings like domain.tld resemble a URL a bit, so it's OK to fix them up, but From\tSubject really doesn't, so we should be a bit more strict in this regard. "Does it look like a URL" is more difficult for computers than humans, but I think we can do better.

Indeed, the URL code strips some whitespace characters, but that flexibility is mostly to improve webcompat and to make things easier for developers. When we get weird stuff from other sources, we should try to avoid surprises.
I'm not really convinced by this approach, but I assume it fixes the issue here.

I actually think that fixing up by ignoring tabs is probably the user-friendly thing to do in general. The combination with dnd to the result that we get here is unfortunate, but fixing only the dnd consumer would feel wrong because it would leave other issues with e.g. copy/paste (or "Paste and Go").

Arai, Boris, what do you think?
Assignee: nobody → gijskruitbosch+bugs
Attachment #8985175 - Flags: feedback?(bzbarsky)
Attachment #8985175 - Flags: feedback?(arai.unmht)
Assignee: gijskruitbosch+bugs → nobody
(In reply to Valentin Gosu [:valentin] from comment #23)
> Indeed, the URL code strips some whitespace characters, but that flexibility
> is mostly to improve webcompat and to make things easier for developers.

I... continue to find this very strange. Anyway, this is not the best place to have that particular discussion.

> When we get weird stuff from other sources, we should try to avoid surprises.

I mean, we do the inverse (but also wrong) thing in e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1460097 in that we don't remove enough whitespace when humans try to load content. Fixup is *meant* to take things that are barely URLs and make them into URLs. That's part of why I'm conflicted (see comment #24 - fwiw, I hadn't seen your comment when I wrote/posted the patch...)
Comment on attachment 8985175 [details] [diff] [review]
Don't fix up protocols for tab-separated content

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

Thanks!

So, this solves the case that there are at least 3 columns (so there are 2 TABs between them) in the mail listing,
which would match default case and most usecases, I think?
if there's no existing testcase that fails with this change (so, no known case that we should handle otherwise), I think this is reasonable fix for this bug.


the another solution would be, do not add prefix/suffix if there's at least one whitespaces in between non-whitespace characters of given text.
the possible case I can think of which there's whitespaces inside the input URL is, when the URL is copied from somewhere else that wrapped the URL with extra whitespaces. so at least suffix should already be there (let me know if there's another case).
this bug's case is troublesome because we add ".com" to non-hostname thing.
we could be more careful/strict when to treat the given text as possible partial hostname.
https://searchfox.org/mozilla-central/rev/d544b118e26422877108c42723b26e9bb4539721/docshell/base/nsDefaultURIFixup.cpp#569-585

::: docshell/base/nsDefaultURIFixup.cpp
@@ +365,5 @@
> +  // or the flags passed might not let us).
> +  rv = NS_OK;
> +  // Avoid fixing up content that looks like tab-separated values (more than 1 \t).
> +  auto firstTab = uriString.FindChar('\t');
> +  if (firstTab == kNotFound || uriString.RFindChar('\t') == firstTab) {

I guess it's better adding a static function with descriptive name (like, MaybeTabSeparatedValues, or something like that?),

also, it should be better handling leading/trailing TABs as well.
currently leading/trailing SPACEs are removed here, but TABs are kept:
https://searchfox.org/mozilla-central/rev/d544b118e26422877108c42723b26e9bb4539721/docshell/base/nsDefaultURIFixup.cpp#150-151

if there are only multiple leading/trailing TABs in the given text (like, "\t\tmozilla.com\t\t\t", it's now treated as non-URL.
So, this should test that there are at least 1 TAB sequence in the middle of non-TAB text.
Attachment #8985175 - Flags: feedback?(arai.unmht) → feedback+
Flags: needinfo?(haftandilian)
Flags: needinfo?(arai.unmht)
Jana/Richard, out of interest, what do you *expect* / *want* from dragging an email to the Firefox tab?
Flags: needinfo?(richard.j.stewart)
Flags: needinfo?(jana.squires)
I don't want anything to happen per se.  Instead, it was merely an observation stumbled upon when attempting to add attach an email to a Jira ticket. Instead of dropping it in the ticket, it was dropped in the tab area; which then created a connection to the malicious url.
Flags: needinfo?(jana.squires)
Same here. I intended to drop an email as a msg file but that's not what outlook gives Firefox. As long as people don't get redirected to a random malicious site I think that's acceptable.
Flags: needinfo?(richard.j.stewart)
For links in web pages, the reason we have to deal with tabs is stuff like this:

  <a href="http://foo.bar.com/foo/
             baz.html">Stuff</a>

Where the indent after the linebreak includes tabs.

It's not clear what should happen if we get a string like that sent to us from an external app...
Comment on attachment 8985175 [details] [diff] [review]
Don't fix up protocols for tab-separated content

Seems reasonable to me....
Attachment #8985175 - Flags: feedback?(bzbarsky) → feedback+
Dan, did you use one of the forms on https://wiki.mozilla.org/Security/Safe_Browsing#Links to report the malicious domain to Google? If so, which one?
Flags: needinfo?(dveditz)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
Valentin, bz is out, could you review this?

Dan, can you add a rating? For me the redirects point to a page that heavily suggests downloading some malware - but doesn't do anything to keep you there / stop you closing the tab. So I don't think this is severe enough to warrant sec-high/sec-crit and therefore sec-approval, but if you disagree I can fill out the form... :-)
Attachment #8985175 - Attachment is obsolete: true
Attachment #8986141 - Flags: review?(valentin.gosu)
Attached patch Add testsSplinter Review
Not sure if we want to land this separately, but just in case, separate patch for the tests.

I got a bit frustrated with the xpcshell lack of messaging about deciphering what:

 0:01.62 FAIL do_single_test_run - [do_single_test_run : 619] false == true


actually meant, so I added some more messages to the tests to make them easier to read. Even without that though, this is enough of a not-a-bullseye that I think we could probably land them at the same time. It's pretty hard to go from this patch (change how we handle \t in URI fixup) to "you should drag emails or other tab-separated content to places to land on malicious pages".
Attachment #8986142 - Flags: review?(valentin.gosu)
Component: Drag and Drop → Document Navigation
Comment on attachment 8986141 [details] [diff] [review]
Patch

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

::: docshell/base/nsDefaultURIFixup.cpp
@@ +133,4 @@
>    return false;
>  }
>  
> +// Assume that 1 tab is accidental, but more than 2 implies this is

Self-nit: more than 1

@@ +135,5 @@
>  
> +// Assume that 1 tab is accidental, but more than 2 implies this is
> +// supposed to be tab-separated content.
> +static bool
> +MaybeTabSeparatedContent(const nsCString& aStringURI)

I would have used nsACString, but RFindChar doesn't exist there... :-(
Attachment #8986141 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8986142 [details] [diff] [review]
Add tests

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

::: docshell/test/unit/test_nsDefaultURIFixup_info.js
@@ +600,4 @@
>        // Both APIs should then also be using the same spec.
>        Assert.equal(!!fixupURIOnly, !!URIInfo.preferredURI);
>        if (fixupURIOnly)
> +        Assert.equal(fixupURIOnly.spec, URIInfo.preferredURI.spec, "Fixed and preferred URI should match");

Nit: also add braces since we're here :)
Attachment #8986142 - Flags: review?(valentin.gosu) → review+
(In reply to François Marier [:francois] from comment #32)
> Dan, did you use one of the forms on https://wiki.mozilla.org/Security/Safe_Browsing#Links

I used the malware one, and I might have also used the phishing one just in case I guessed wrong about how Google would categorize it.

I'm guessing sec-low for the rating because Firefox ought to be robust in the face of a malicious site since there's any number of ways users can stumble into one. But depending on what site comes up and what users /thought/ they were doing by dragging the mail our behavior might contribute to social engineering attacks.
Flags: needinfo?(dveditz)
Keywords: sec-low
https://hg.mozilla.org/mozilla-central/rev/b58c103b639f
https://hg.mozilla.org/mozilla-central/rev/34fc2ff698c3
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
I'm kinda-sorta wondering if we want to take this on ESR60 or not given the attack vector involved. It grafts cleanly. Thoughts?
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8986141 [details] [diff] [review]
Patch

(In reply to Ryan VanderMeulen [:RyanVM] from comment #40)
> I'm kinda-sorta wondering if we want to take this on ESR60 or not given the
> attack vector involved. It grafts cleanly. Thoughts?

Sounds reasonable to me. We have decent automated test coverage here, and this has been sitting on beta for a while now without (AFAIK) complaints.

[Approval Request Comment] (for both bugs)
If this is not a sec:{high,crit} bug, please state case for ESR consideration: workaround-ish fix for issue with workflows that may be particularly prevalent on ESR
User impact if declined: potential scam site access
Fix Landed on Version: 61
Risk to taking this patch (and alternatives if risky): low. Small patch on code that has decent unit tests.
String or UUID changes made by this patch: none.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8986141 - Flags: approval-mozilla-esr60?
(In reply to :Gijs (he/him) from comment #41)
> [Approval Request Comment] (for both bugs)

both *attachments*. Sigh. Need my tea in the morning before commenting on bugs, clearly...
Comment on attachment 8986141 [details] [diff] [review]
Patch

Fixes a sec issue with links dragged from Outlook (which seems more likely in managed environments that are also more likely to be running an ESR), well baked on 62, and includes new tests. Approved for ESR 60.2.
Attachment #8986141 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main62+][adv-esr60.2+]
(In reply to Robert Neumann from bug 1480734 comment #1)
> The exact same issue was discussed in this very old article:
> 
> https://www.silverspider.com/2007/from-subject-received-size-categories/

Seems like this is already public, should we unhide the bug here? Dan?
Flags: needinfo?(dveditz)
We had a quick blog on it a few weeks back as well.

https://www.forcepoint.com/blog/security-labs/subject-received%E2%80%A6what

Better transparency at this point might be reasonable.
Group: core-security-release
Flags: needinfo?(dveditz)
Alias: CVE-2018-12381
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: