Closed Bug 1810793 Opened 1 year ago Closed 1 year ago

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

Categories

(Firefox :: File Handling, defect)

defect

Tracking

()

VERIFIED FIXED
112 Branch
Tracking Status
firefox-esr102 --- fixed
firefox110 --- wontfix
firefox111 --- wontfix
firefox112 --- fixed

People

(Reporter: ameenbasha111, Assigned: enndeakin)

References

(Regressed 1 open bug)

Details

(Keywords: reporter-external, sec-low, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main112+])

Attachments

(5 files, 1 obsolete file)

Attached file 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

Flags: sec-bounty?
Attached file background.js

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

Component: Security → General
Flags: needinfo?(ameenbasha111)
Product: Firefox → WebExtensions
See Also: → CVE-2023-25734

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

Flags: needinfo?(ameenbasha111)

POC Video Attached

Attached file 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

  1. 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

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

  3. add the extension. Click the extension.

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

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.

(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.

Thanks for the update.

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?

Flags: needinfo?(rob)

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.

Keywords: sec-low

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

(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..

Flags: needinfo?(rob)

(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?

Flags: needinfo?(rob)

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

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.

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

(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.

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.

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.

Duplicate of this bug: 1814701

(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.

Flags: needinfo?(rob)
No longer duplicate of this bug: 1814701

@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.

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

Summary: FireFox Extension API : .url extension Download leads to Arbitrary File Read → FireFox Extension API : .url / .lnk / .local file download lead to Multiple issues

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 1814701

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

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

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.

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.

Flags: needinfo?(rob)

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.

Component: General → File Handling
Flags: needinfo?(rob)
Product: WebExtensions → Firefox

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

(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.

Depends on: CVE-2023-29542

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

its been a month since i raised this issue

(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.

@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

(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 security@mozilla.org - but I don't believe that is the case for any of the tickets you pinged.

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

(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.

  1. 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.

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

Flags: needinfo?(rob)

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.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(rob)
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
See Also: → 1819643
Assignee: nobody → enndeakin
Group: firefox-core-security → core-security-release
Target Milestone: --- → 112 Branch
Flags: sec-bounty? → sec-bounty+

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

(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?

Flags: needinfo?(tom)

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

Bounty questions can be emailed to security@mozilla.com

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

Flags: needinfo?(tom)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main112+]

(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

Attached file advisory.txt
Attachment #9327441 - Attachment is obsolete: true
Regressions: 1827115

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

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.

Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: