1543191 - (CVE-2019-11698) Stealing history data using places querying and drag and drop

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Abdulrahman Alqabandi]

Attachment: poc.htm

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Using attached PoC:

1. If you don't have bookmarks bar open then open sidebar bookmark page by pressing CTRL+B
2. Drag and drop the anchor tag into the bookmark bar/sidebar
3. Now drag and drop the newly created bookmark into the webpages body

Actual results:

For some reason the dragged object from the bookmark bar contains 'text/html' formatted data of all the queried history entries. Which means we can steal users history data (even loaded iframes) with precision, thanks to the use of the places query.

Step [2] creates a places query ( https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Places/Places_query_URIs ) that looks like:

place:type=0&sort=1&terms=token&maxResults=10&includeHidden=true

I am looking within the users history for any entry with the term 'token' in the URL, for testing purposes I only grab 10 entries but this could be increased in an attack scenario. I also use the 'includeHidden' query operator to include otherwise hidden history entries that come from iframes and images, just to further expand my net.

Step [3] is where I think the bug exists. Once we drag the newly created places query bookmark back into our webpage, we can read all the results from our query by reading the 'text/html' data entry.

Expected results:

There should not be a ''text/html' readable data when drag and dropping bookmarks from firefox and to firefox. I believe this may have been put there for cross application compatibility but is of no use (AFAIK) when it comes to drag and dropping within firefox.

Comment 1

[Abdulrahman Alqabandi]

Attachment: 2019-04-09 18-58-54.mp4

Poc video

Comment 2

[:Gijs (he/him)]

Marco, how hard would it be to only provide the text/html flavour for non-Firefox, or system privileged bits of Firefox or something?

Comment 3

[Marco Bonardo [:mak]]

Thank you for the nice bug report!

(In reply to :Gijs (he/him) from comment #2)

Marco, how hard would it be to only provide the text/html flavour for non-Firefox, or system privileged bits of Firefox or something?

First because when you start a drag&drop you can't tell where the drop will happen, so you can't really exclude anything. You just have to put all the info in the dataTransfer object and let the consumer handle it.
Second the user may be willing to drop into a text file or a third party app, that is indeed a feature we wanted.

I think we may go the opposite way, disallow dropping a place: url on a Places view if the only available flavor is text, we have a Places flavor for chrome to chrome drops, so there's no reason we should allow to drop a place: url from a text flavor.

Comment 4

[Marco Bonardo [:mak]]

Obviously the above solution will disallow the page from creating specific queries, it will still be possible to drop an existing query... Though we don't create anymore smart queries, so it's likely only advanced users have some, and those are unlikely to fall for this kind of trick. The only remaining thing you could drop is folders or Bookmark roots, it would be interesting to see which social skills one should use to convince a user to do that. It would still be possible anyway, and it would disclose the list of bookmarks in that folder/root. I'm not sure how to solve that while still allowing the possibility to drop a list of urls to a third party app.

Comment 5

[Mark Banner (:standard8)]

Something I've just discovered is the mozSourceNode property on DataTransfer elements. I tested it briefly and it gives me the browser element for the web page in this case.

Could we do some sort of filtering based on that, e.g. checking the source and if it is content then disallow x-moz-place? (might need to make sure we still allow it for webextensions).

Comment 6

[Marco Bonardo [:mak]]

Let's sum up:

1. We can disallow extracting place uris from MOZ_URL and UNICODE flavors, I'll file a separate bug to do this. This should disallow dragging or copying simple place: links from a page.
2. For drag&drop, we may check mozSourceNode to verify if the source is not Firefox chrome... But we can't do that for copy & paste because nor nsIClipboard, nor nsITransferable give us any info about the data source. So potentially a page may still create a dataTransfer or populate the clipboard with a MOZ_PLACE flavor. We need a way to check if the clipboard contents come from Firefox chrome or somewhere else.
3. In any case if the user drags or pastes an existing container to content, we can't do much, unless we blacklist place flavors from reaching content (not sure how to do it, but it should be possible) and remove the possibility to drop containers on third party apps. This path is less data sensitive anyway because, if we fix 1) and 2), convincing the user to create a place query exposing all of history would be quite a challenge. Moreover, if we disallow place flavors from a second Firefox instance, the only way to support moving stuff across multiple instances will be the unicode part, that we already filtered out.

Basically, we need a way to:
a. Figure out the source in copy & paste case, is it Firefox chrome or something else? We should be able to accept WebExtensions generated data though. I think distinguishing chrome from content would be enough.
b. optionally blacklist certain flavors for content (so it can't add place flavors to the clipboard or a DataTransfer)

Neil, any idea of whether a), b) are currently possible from js with the existing APIs, and how complicate it would be to add them?

Comment 7

[Marco Bonardo [:mak]]

Note, we have another case that Mark is looking into, where we would like to know if the clipboard data comes from this instance of firefox or another one. So a solution supporting both cases would probably be the best. the DataTransfer mozSourceNode property seems ok, it just seems to not be available in the c&p case.

Comment 8

[Neil Deakin]

For b) you can block certain types from being added from content by adding them to DataTransfer::PrincipalMaySetData.

For a) I'm not clear from the description above what you're wanting to prevent. Could you provide the steps, or perhaps this is another bug referenced by comment 7?

Comment 9

[Marco Bonardo [:mak]]

Thanks, I'll file a separate bug to prevent place flavors from being added by content in PrincipalMaySetData.

(In reply to Neil Deakin (away march 22 - 31) from comment #8)

For a) I'm not clear from the description above what you're wanting to prevent. Could you provide the steps, or perhaps this is another bug referenced by comment 7?

I assume content may potentially create any kind of flavor, when we have an nsITransferable we know what it contains, but we don't know in which context it was created. The steps is:

1. content puts in the clipboard some data with a places flavor
2. somehow it convinces the user to paste it into the bookmarks toolbar
3. because the flavor and data is valid we do the action

It's true that by blocking the moz-place types from being added, as you suggested, we can prevent this, but I would feel safer if I could say "this data has been added by content, so I don't want to handle it", mostly because people tend to forget to update blacklists. It would be a gate check.

The other case we have, is that we need to recognize if the data comes from current Firefox or another instance of Firefox, because for example it may contain a bookmark with a GUID that is valid for the other instance but not for this one. We can't check all the guids because it would be really expensive (suppose the user dragged a thousand bookmarks), so we were thinking to use mozSourceNode, but that exists only for D&D, not for C&P.

Comment 10

[Neil Deakin]

(In reply to Marco Bonardo [::mak] from comment #9)

It's true that by blocking the moz-place types from being added, as you suggested, we can prevent this, but I would feel safer if I could say "this data has been added by content, so I don't want to handle it", mostly because people tend to forget to update blacklists. It would be a gate check.

The data in a DataTransfer is stored with the principal of the caller that put each item there, so that would probably be the route to handling this case. This information isn't passed on to the Transferable though, and I'm not sure if it currently gets preserved across process boundaries either.

The other case we have, is that we need to recognize if the data comes from current Firefox or another instance of Firefox, because for example it may contain a bookmark with a GUID that is valid for the other instance but not for this one. We can't check all the guids because it would be really expensive (suppose the user dragged a thousand bookmarks), so we were thinking to use mozSourceNode, but that exists only for D&D, not for C&P.

Essentially, to do this properly, you would need to know if the system clipboard is currently assigned by Firefox, and to know if another application takes ownership of the clipboard for its own data. This is possible, but we haven't had any need for this information so far, so would require some platform-specific work to implement.

Comment 11

[Daniel Veditz [:dveditz]]

I can't quite call this sec-high given the user interaction required, but this is pretty bad. It seems to be the bug is step 2 when we let a page create drag information about a URL it can't actually navigate to. Can we annotate drags with a "from untrusted content" flag so we can prevent that kind of thing?

Comment 12

[Daniel Veditz [:dveditz]]

Going to go with sec-high after some debate. the places search language means that if you could find a willing victim you could mine some very specific information about them.

Comment 13

[Marco Bonardo [:mak]]

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

I can't quite call this sec-high given the user interaction required, but this is pretty bad. It seems to be the bug is step 2 when we let a page create drag information about a URL it can't actually navigate to. Can we annotate drags with a "from untrusted content" flag so we can prevent that kind of thing?

The dependencies I filed, and that I'm patching, disallow pages from adding place: uris to d&d or c&p flavors, though we can't really disallow a user from dragging already existing bookmarks/history stuff to content, so you can potentially drag your Bookmarks Toolbar root or your history Today's container to content. I'm not sure how to disallow that without completely removing whole data exchange features that the user may want (like taking a whole list of urls to a third party app, and probably even a Web App).

Regarding the possibility to check drags from untrusted content, we can, but only for drag&drop, copy&paste doesn't seem to expose that info to javascript for now (that's what I was asking to Neil).

Comment 14

[Marco Bonardo [:mak]]

One side of this (allowing content to create a custom place: query) is now fixed in Nightly and I asked for uplift.

The other sides are more complex:

1. provide a way to tell from js whether the origin of the data was chrome; possible for d&d through mozSourceNode, not possible for the clipboard without platform work. This is not strictly necessary for this specific bug, because the dependencies are enough, but worth considering for the future. Bug 1543954 tracks a request to get more information regarding the clipboard.
2. disallow dropping Places containers to content; this is complex because one of the features of D&D and the clipboard is to share data with other apps, so a user may want to actually get the list of urls in a container, and currently he can just c&p or d&d the container to whatever third party app he wants, this includes Web Apps. Even if we'd disable dropping Places specific flavors to content, we'd still provide the same information in text flavors. I'm not sure how we could fix this without dropping the feature.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

mak, can you describe how QA might be able to verify bug 1543617? Either here (in which case I'll cc someone into the bug) or in the public facing issue.

Comment 16

[Marco Bonardo [:mak]]

Trying to reproduce this bug is enough, the technical details are not that important for QA to verify it, imo (also because it would require to build a testcase identical to what we have in automation).

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Bug 1543617 and bug 1543694 are fixed/verified (in 68/67 and likely soon for ESR as well).

Should we mark this as fixed and open a new sec issue (if necessary) to keep track of whatever is left over in bug 1543954?
If we keep it open, we need to still make sure to check if we need to write a sec advisory for the issues fixed.

Comment 18

[Al Billings [:abillings - ex-MoCo]]

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #17)

Should we mark this as fixed and open a new sec issue (if necessary) to keep track of whatever is left over in bug 1543954?
If we keep it open, we need to still make sure to check if we need to write a sec advisory for the issues fixed.

We should open a new bug to keep this one as clean as possible.

Comment 19

[Marco Bonardo [:mak]]

I agree, let's close this, I'll file a new bug to check the origin of drops. I'm not sure whether it's worth filing a bug for the possibility to drag an existing container to content, as I said that's actually a feature.

Comment 20

[Marco Bonardo [:mak]]

I filed bug 1545840 for checking origin of paste/drops. We can't do that without platform support, anyway.

Comment 21

[Daniel Veditz [:dveditz]]

Going back to my first instinct (sec-moderate) that this exploit requires more user interaction than we consider for sec-high.

Comment 22

[Bogdan Maris, Desktop QA]

Reproduced the initial issue described in comment 0 using an old Nightly from 2019-04-09. Verified that the issue does not reproduce anymore using the steps provided in comment 0 on latest Beta 67.0b13 and latest Nightly 68.0a1 across platforms (Windows 10 64bit, macOS 10.13 and Ubuntu 18.04 64bit).

Is this sufficient to mark this bug as verified fixed or do we want to keep it as is until bug 1545840 and bug 1543954 will be fixed?

Comment 23

[Marco Bonardo [:mak]]

Imo, we may consider this verified fixed, because it's no more possible to create a custom history view, the remaining bugs are tracking possible future improvements that will be done once it's possible (platform support), but they are just a second line of protection, that would be nice to have but it's not necessary to fix this specific case.

Comment 24

[Bogdan Maris, Desktop QA]

Also verified on esr60.8 and based on comment 23 I'll close this one as verified fixed.