1810793 - FireFox Extension API : .url / .lnk / .local file download lead to Multiple issues

Status: VERIFIED FIXED

(Firefox :: File Handling, defect)

Description

[Ameen]

Attachment: manifest.json

Chromium Bug Reference: https://bugs.chromium.org/p/chromium/issues/detail?id=1307930

HI team i have already reported the same in another Place. Now i have found a alternate way to achieve the Same. Kindly have a look in to my below issue for POC Video.

Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1809923

While visiting a webpage if the download initiate for .lnk/ .local these file types are changed to .download by firefox. But i have found a way to download .lnk/.local/.url Files while lead to Arbitrary File Read etc.( based on file extension)

We can use browser.downloads.download API to download the .lnk/.local/.url file without changing it to .download

FIREFOX VERSION : 108.0.2 (64-bit)

REPRODUCTION CASE:

1. Add the Manifest File to the Extension in about:debugging#/runtime/this-firefox (keep backgroud.js file in same extension folder)
2. Once added Click on the extension
3. You can find the safe.url file is downloaded without renaming

Note: As stated in my previous issue the downloaded .url file can be used to achieve Arbitrary File Read

Same is possible for .lnk and .local too. tested

Comment 1

[Ameen]

Attachment: background.js

Comment 2

[:Gijs (he/him)]

The example background script uses .url, can you provide one that uses .lnk that you say also reproduces the issue?

Comment 3

[Ameen]

Kindly change the filenamed as safe.url in background.js to safe.lnk you can find safe.lnk file is downloaded.

Hope this is what you expected. Kindly respond if any further

Comment 4

[Ameen]

Attachment: Firefox Extension API Exploit.mp4

POC Video Attached

Comment 5

[Ameen]

Attachment: TestExtension.zip

I have Zipped the extension folder which i used in POC Video for Downloading .lnk file

POC Steps:

1. Host the Actual Shortcut file anywhere. i used my own python server. i have attached the same for your reference

Run the Server by running below command in CMD:
python server.py

2. Note down the URL in which the Server is UP:
   eg: 192.168.0.103:5000 (or) you can use 127.0.0.1:5000

3. Replace the Server IP in Background.js download Method.

4. add the extension. Click the extension.

5. You can find the .lnk file is downloaded and the notepad is opened

Comment 6

[Ameen]

Query: why the component is changed to General?

Im new to this firefox bug bounty, i usually hunt on hackerone. here how can i find my issue is triaged and eligible for bounty. What are the process.

Comment 7

[Frederik Braun [:freddy]]

(In reply to Ameen from comment #6)

Query: why the component is changed to General?

WebExtensions:General means, that it is supposed to be fixed by the WebExtensions team

Im new to this firefox bug bounty, i usually hunt on hackerone. here how can i find my issue is triaged and eligible for bounty. What are the process.

The bounty team usually look at bugs once they have been completely fixed, in order to make sure that we're assessing the security implications correctly. Our decision will be reflected in the sec-bounty? flag in this bug. sec-bounty+ means bounty awarded, sec-bounty- means not awarded. If someone is being awarded a bounty, they will get an email in the coming days.

Comment 8

[Ameen]

Thanks for the update.

Comment 9

[Daniel Veditz [:dveditz]]

If you have given the extension permission to download files, is it really a problem that it has downloaded files? If the extension is malicious it will do bad things, and extensions are allowed to do more than a web site which is why users have to agree to give them more permissions. I'm not sure what the Web Extension community expects to happen here.

Is the Extension download API not adding the Mark of the Web? It should, right?

Is the Extension API supposed to do anything different than regular download code? That is, if we fix the earlier bug in shared code, does this one automatically get fixed? At least with respect to .lnk/.local apparently not because the behavior is different -- maybe not using shared code is part of the problem?

Comment 10

[Daniel Veditz [:dveditz]]

For now calling it sec-low because it seems less bad than bug 1809923: the effects are more or less the same, but you have to first be convinced to install an extension and grant extra privileges.

Comment 11

[Ameen]

If you have given the extension permission to download files, is it really a problem that it has downloaded files? If the extension is malicious it will do bad things, and extensions are allowed to do more than a web site which is why users have to agree to give them more permissions. I'm not sure what the Web Extension community expects to happen here.

Have already tested the same in chromium Extension API and it is already fixed. So the same should be handled here too.

Just fyi the dangerous extension download should be prevent by any sort of way. the ultimate goal is to make sure the browser doesn't allow to download these extensions. alternatively it should be renamed to .download

I thought of sharing this info(chromium similar issue handled) could help you guys in some way.

Thanks

Comment 12

[Rob Wu [:robwu]]

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

If you have given the extension permission to download files, is it really a problem that it has downloaded files? If the extension is malicious it will do bad things, and extensions are allowed to do more than a web site which is why users have to agree to give them more permissions. I'm not sure what the Web Extension community expects to happen here.

Downloading itself is not an issue. If there is dangerous behavior, there should be a prompt before opening it, e.g. via MOTW on Windows. There is even an API to launch downloaded files (browser.downloads.open()) that requires the user to consent via an extension permission, and further user interaction to permit the API call.

Is the Extension download API not adding the Mark of the Web? It should, right?

Last time I checked, it should, as elaborated at https://bugzilla.mozilla.org/show_bug.cgi?id=1654819#c4
If the MOTM is missing, then we have to investigate that.

Is the Extension API supposed to do anything different than regular download code? That is, if we fix the earlier bug in shared code, does this one automatically get fixed? At least with respect to .lnk/.local apparently not because the behavior is different -- maybe not using shared code is part of the problem?

Firefox desktop's download implementation is shared with regular downloads. The filepicker initialization is extension-specific (see nsIFilePicker instantiation in ext-downloads.js). This is a Save-as file picker, not an Open-file picker, so that difference in implementation is not significant, as the "exploit" requires the user to select the previously downloaded file in an Open-file picker..

In both this and bug 1809923, there is no "arbitrary file read" until the user is tricked into selecting the downloaded file with a second file picker..

Comment 13

[:Gijs (he/him)]

(In reply to Rob Wu [:robwu] from comment #12)

Firefox desktop's download implementation is shared with regular downloads. The filepicker initialization is extension-specific (see nsIFilePicker instantiation in ext-downloads.js). This is a Save-as file picker, not an Open-file picker, so that difference in implementation is not significant, as the "exploit" requires the user to select the previously downloaded file in an Open-file picker..

In both this and bug 1809923, there is no "arbitrary file read" until the user is tricked into selecting the downloaded file with a second file picker..

OK. So it sounds like in this bug we should change the webextensions code that opens the save-as filepicker to apply the same filename sanitization that the "normal" downloads code does and picks a .download filename by default. Right?

Comment 14

[Ameen]

Hi Guys(Rob Wu). Can you have a look into this.

Comment 15

[Ameen]

Team. The impact was much more. that is without user interaction we can get their NTLM hash.
See the below comment for More details.

https://bugzilla.mozilla.org/show_bug.cgi?id=1809923#c17

Kindly Initiate the quick fix on this case.

Comment 16

[Ameen]

Since the impact is Same to issue 1809923. i have shared its comments no 17.

Comment 17

[:Gijs (he/him)]

(In reply to Ameen from comment #15)

Team. The impact was much more. that is without user interaction

Without user interaction you can't install an extension.

Comment 18

[Ameen]

Hi Gijs, Yes for installing Extension we need the user interaction. The thing which i was try to convey is. After downloading the .url file there is no interaction needed it will leak the ntlm hash without interaction.

i.e: install the extension(user interaction needed) -> now when extension executes it leaks the ntlm hash.

Hope this clears. Sorry for the confusions if any.

Comment 19

[Ameen]

hey guys, is the fix started for this issue?

Since there is no update on this issue regarding fix / any progress so just asked for an update.

Also to keep this in track and deploy a quicker fix can we have the severity and priority sets for this issue.

Comment 21

[Rob Wu [:robwu]]

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

(In reply to Rob Wu [:robwu] from comment #12)

Firefox desktop's download implementation is shared with regular downloads. The filepicker initialization is extension-specific (see nsIFilePicker instantiation in ext-downloads.js). This is a Save-as file picker, not an Open-file picker, so that difference in implementation is not significant, as the "exploit" requires the user to select the previously downloaded file in an Open-file picker..

In both this and bug 1809923, there is no "arbitrary file read" until the user is tricked into selecting the downloaded file with a second file picker..

OK. So it sounds like in this bug we should change the webextensions code that opens the save-as filepicker to apply the same filename sanitization that the "normal" downloads code does and picks a .download filename by default. Right?

The Save-as file picker is not relevant. The downloads API can be used to download to a file name of choice (within the Downloads directory), without further user interaction other than having the extension installed. They can do so by setting saveAs: false to skip the Save-as dialog.

The bug report here is overstating the issue: it claims arbitrary file read, which is not the case. What it shows is that an extension can download an arbitrary resource and then the user can open it with user interaction. I would rate that bug as invalid. Downloading arbitrary URLs is an explicit feature of the download API.

Bug 1814701 would be closer to a valid bug report. I'm going to change the duplicate order, since the other bug is more actionable than this one.

Comment 22

[Ameen]

@Rob Wu i dont understand why it is considered as invalid bug and the other one is closer to valid one.

In this case i have shown that we can download .url/.lnk/.local files too. which is usually a dangerous files in windows

And also for a impact

1. With user interaction we can read files
2. Without user interaction just by downloading the url/.lnk file it leaks the user NTLM hash. (regarding this you can check my another report which shows this impact too)

There is some misunderstanding at your end. Have a look into this issue.

I have provided the necessary information for the issue from the beginning.

Comment 23

[Ameen]

Also fyi: the .lnk / .local are already fixed in chrome.downloads.download too.

I have Raised a separate ticket to Chrome to handle the .url too.

But in this case all 3 are acceptable

Friendly reminder to Re-Check my issue and assign it to the respective person for a fix

Comment 24

[Rob Wu [:robwu]]

I'm going to mark the other bug as a duplicate of this. Given the similarities, I previously thought that you were the author of the other bug, but that was not the case. To keep the discussion in one bug, here is my comment that I posted in the other bug. I'll discuss this bug with my team.

The ability to choose a file name is a feature. Extensions can currently download to an arbitrary file name or directory within the (user-configured) Downloads.

Being stricter on the file names or even changing their extension may not be backwards-compatible, but if there is a good reason to, we could consider that.

For comparison, I checked Chrome's behavior.
In the far past (e.g. Chrome 40), it prompted about dangerous files, to either keep or discard it (without creating the file in the destination).
These days, Chrome appears to change the file extension. E.g. if foo.sh is passed, foo.txt is created instead.

Comment 26

[Ameen]

Hi Rob Wu.

but if there is a good reason to, we could consider that.

The extensions which are all considered as dangerous should be blocked in all possible ways.

In this case Firefox also blocked / append the .download to .lnk/.url/.local extensions in general downloads.

Meantime chrome downloads api also don't allow to download those extensions. it will change them to .download (this is because the extension are having some special functionality which can be trick/ change the execution path)

Since firefox already restricts the normal download the same should be handled here too to avoid the same impact.

Also just fyi: .url / .lnk file leaks the user NTLM hash just by downloading without any clicks

Thanks

Comment 27

[Ameen]

Guys its been 20 days since i raised this issue, Can we have a quick fix for this.

Comment 28

[Tomislav Jovanovic :zombie]

We're still evaluating solutions for this bug, and don't consider it a critical issue that needs to be fixed ASAP. Please wait for the completion of our normal process, we will inform you of any updates we might have.

Comment 29

[Rob Wu [:robwu]]

We're thinking of letting ext-downloads.js call mimeService.validateFileNameForSaving behind a pref (on by default), and potentially remove pref later (after ESR) if there are no reported regressions from extension developers.

That would make the filename sanitization behavior of extensions more consistent with Firefox's usual download functionality.

needinfo to me to look into this further over the next few weeks and to schedule the implementation effort.

Comment 30

[Rob Wu [:robwu]]

I took a look, and it looks like we are already relying on validateFileNameForSaving via DownloadPaths.sanitize:

• source to validate when the filename is specified by the extension.
• source to sanitize the filename derived from the URL.

The implementation of DownloadPaths.sanitize specifies the VALIDATE_SANITIZE_ONLY flag, which triggers a simplified code path (using the SanitizeFileName method) that excludes the part that adds .download for .lnk etc..

While I can fix the extension-specific use by directly calling validateFileNameForSaving with different flags, I think that that the fix should be more general considering that the helper is used in more places outside the realm of WebExtensions: https://searchfox.org/mozilla-central/search?q=downloadPaths.sanitize&path=

• validateFileNameForSaving should check the special filenames (.lnk, etc) even with VALIDATE_SANITIZE_ONLY,
• or validateFileNameForSaving should take a new flag that does it, and DownloadPaths.sanitize should pass the flag.

... since this work is not in the extensions framework, I'm moving this to Firefox::File Handling. An alternative component could be the home of the DownloadPaths module, i.e. Toolkit::Downloads API.

Comment 31

[Neil Deakin]

The patches in bug 1815062 cover the changes described here in comment 30.

Comment 32

[Rob Wu [:robwu]]

(In reply to Neil Deakin from comment #31)

The patches in bug 1815062 cover the changes described here in comment 30.

In the current version of the patch, the new allowInvalidFilenames flag defaults to true. I would expect false there - every caller should receive a sanitized filename.

Comment 33

[Ameen]

Team the issue is still unassigned can anyone have a look into it

its been a month since i raised this issue

Comment 34

[:Gijs (he/him)]

(In reply to Ameen from comment #33)

Team the issue is still unassigned can anyone have a look into it

its been a month since i raised this issue

It's not acceptable to repeatedly comment on issues every week or whatever, just to "ping" them. Please stop doing this.

In any case, as the comments already indicated, a fix is in progress in a separate linked bug on which this bug is marked as dependent, so the issue is actually being looked at.

Comment 35

[Ameen]

@Gijs

I haven't asked for update in past some days. also i have asked for an update because of the below mentioned comment. which state the issue is still unassigned that means the fix is not started yet

So to avoid unnoticed. i have pinged in the tickets an hour back.

FYI: my last reply on this ticket is about 17 days back.

https://bugzilla.mozilla.org/show_bug.cgi?id=1813299#c11

Sorry for the inconvenience if any

Comment 36

[:Gijs (he/him)]

(In reply to Ameen from comment #35)

also i have asked for an update because of the below mentioned comment. which state the issue is still unassigned that means the fix is not started yet

That's not what the comment says. Also the comment is by a bot, and on a completely different bug, which someone tracked for a specific release. This bug is not tracked in that way (and marked sec-low), so finding an assignee is not as urgent.

So to avoid unnoticed. i have pinged in the tickets an hour back.

And I'm telling you, please don't do that - certainly not for tickets like this one where there is already a discussion on the fix happening on a different bug, or which are marked sec-low - which is in fact the case for all but 1 of the tickets you pinged.

The only people it'll ping is people already copied on the ticket who happen to have emails turned on for comments (which many people don't, so then they just don't notice and you don't achieve your aim of drawing attention to the ticket anyway).

If the people copied on the ticket thought that particular ticket should be at the top of their list, it would be there already. If one of your reports with a high/critical severity is going unaddressed for a month+ at a time or something without any explanation in the comments of that bug, needinfo the triage owner specifically, use chat.m.o as you were told elsewhere, or email [e-mail] - but I don't believe that is the case for any of the tickets you pinged.

Comment 37

[Ameen]

Okey Gijs Thanks for the update. one final query

1. In general how long will it take for sec-low issue to get fix.
2. usually on which interval i can asked for an update on any sec-low issues

Comment 38

[:Gijs (he/him)]

(In reply to Ameen from comment #37)

1. In general how long will it take for sec-low issue to get fix.

That's highly variable but generally they're not super high priority compared with sec-high/sec-crit or other issues that affect a lot of users.

2. usually on which interval i can asked for an update on any sec-low issues

I would suggest you don't, and focus on higher severity issues, unless something about the bug other than "time passed" has changed materially - e.g. someone has publicized the issue elsewhere, you're aware of it being actively exploited by malicious actors, it was fixed or also discovered in other browsers, you found a variation on the issue that warrants more serious investigation, etc.

Comment 39

[:Gijs (he/him)]

I think this should be fixed by Neil's patch in the dep, in current 112 nightly. Rob, can you confirm?

Comment 40

[Rob Wu [:robwu]]

Confirmed.

When I test the following snippet from an extension with the "downloads" permission,

browser.downloads.download({
  url: URL.createObjectURL(new Blob(["x"])),
  filename: "x.url",
  saveAs: true, // or similarly, without this line.
});

In Firefox 111 and earlier:

• A file called "x.url" was created.

With the changes from https://hg.mozilla.org/mozilla-central/rev/642008166aacd5c65b428365bdef287c4ee3bc15:

• "Error: filename must not contain illegal characters"

The error message is slightly inaccurate, but if desired we could fix that in a separate follow-up (or at least add a unit test eventually).
For now, I think that no further action is needed.

Comment 41

[Ameen]

Team still i haven't received mail for the bounty process for this issue, can someone initiate this?

Comment 42

[:Gijs (he/him)]

(In reply to Ameen from comment #41)

Team still i haven't received mail for the bounty process for this issue, can someone initiate this?

Tom, are you the right person to look into this?

Comment 43

[Ameen]

Also kindly share the CVE-ID details registered for this issue.

Comment 44

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Bounty questions can be emailed to [e-mail]

A CVE will be assigned when this version is in the week before release. Firefox 112 ships on April 11

Comment 45

[Ameen]

(In reply to Tom Ritter [:tjr] from comment #44)

A CVE will be assigned when this version is in the week before release. Firefox 112 ships on April 11

As per this comment firefox 112 ships in 4-5 days. Can i get the CVE ID details

Comment 46

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt

Comment 47

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt

Comment 48

[Ameen]

https://www.mozilla.org/en-US/security/advisories/mfsa2023-13/#CVE-2023-29542

Team kindly update this CVE Id to this ticket. seems it was missed to update here and only updated in security advisory link

Comment 49

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Because of the way Bugzilla works, we can only give the CVE alias to one of the bugs. However, the alias used in bugzilla is fairly meaningless - when the CVE details make their way into the MITRE database, it will reference both bugs in all mechanisms that display CVE details.