1813299 - (CVE-2023-37206) Arbitrary File read via social engineering and re-uploading directories containing symbolic links

Status: RESOLVED FIXED

(Core :: DOM: File, defect)

Description

[Ameen]

Hi team. While looking for Firefox File upload feature and digging more about my another report (.desktop/.symlink) download. i have found a interesting exploit which can lead you to upload Arbitrary File.

Note: Chrome Fix this issue already

Ref: https://bugs.chromium.org/p/chromium/issues/detail?id=1345275

Root Cause: While opening/uploading a file firefox follows the symbolic link which allows to upload arbitrary file (many of the user doen't aware about the symbolic link so major of the victim will fall on this.)

Mac also support the symblink so i hope the same is vulnerable on Mac too.

POC:

1. Open the Html file
2. unzip the downloaded file.
3. Upload the folder
4. Boom Some other arbitrary files are uploaded.

Note: I have attached the POC files used in the Video (Also attached the POC Video)

In the POC Video i have shown the same with chrome. which was already fixed.

Comment 1

[Ameen]

Attachment: Symlink-POC.zip

Comment 2

[Ameen]

Attachment: Symlink-FileRead.mp4

Comment 3

[Ameen]

FYI: the .wave folder contains the files which has symblink to the victim machine. so uploading the folder uploads the symlink mapped files recursively.

Comment 4

[Ameen]

Still the issue is not assigned to any of the developers/assignee. Kindly assign it to respective teams for quick fix on this

Comment 5

[Ameen]

It is still in unassigned state, can we get this assigned to developers.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sounds like a file/folder picker's issue under widget/ (I recalled bug 271732).

Comment 7

[Ray Kraesig [:rkraesig]]

This is not a Windows issue, or at least the provided POC doesn't apply on Windows (and not just for trivial reasons like "the paths referenced in these symlinks don't exist"). The Windows shell's built-in ZIP extractor discards the symbolic-link nature of these files and turns them into flat text.

(Windows does support symlinks, but by default, you need elevated permissions to create them, and anyway having those permissions does not change the built-in extractor's behavior. I believe 7Zip will create symlinks if you have permissions, but these symlinks are detected as dangerous and not created.)

Forwarding on to DOM: File, in the hopes of a cross-platform solution.

Comment 8

[Ameen]

To avoid confusions on your end. This will not applicable on windows platform.

it Will be applicable to linux (ubuntu, kali linux etc)based environments and Mac OS

Comment 9

[Ameen]

Hi guys is there any progress update on this? that is Fix is already started?

Comment 10

[Jari Jalkanen]

[Tracking Requested - why for this release]:

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox111 (beta) and tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in 14 days. However, the bug still isn't assigned.

:jstutte, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 12

[Jens Stutte [:jstutte]]

Andrew will come up with a plan here.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox111 (beta) and tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in 8 days. However, the bug still isn't assigned.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox111 (beta) and tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in 7 days. However, the bug still isn't assigned.

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

The tl;dr general situation here is:

• GetFilesHelper will intentionally include symlinks both when specifically selected and when recursively walking a directory.
  • We intentionally enabled this in bug 1274959 with :smaug I think making the policy decision in bug 1274959 comment 1.
  • The bug did add logic to avoid creating infinite loops when it relates to symlink traversal of directories.
  • Recursive directory traversal also does not pay attention to conventions about dot-files potentially being hidden.
• Our File/Blob abstractions as returned by the directory picker create the stream without passing O_NOFOLLOW or otherwise doing anything that would cause an error when we read the contents of the File. We have no ability/desire to expose "hey, this is a symlink" to content via spec semantics.
• The general attack model described in this bug:
  • Convince a user to extract an archive which includes symlinks inside a directory hierarchy.
  • Convince the user to then upload the portion of the hierarchy that includes the symlinks, which we will traverse. Because the symlink can point to anywhere on any path that is accessible to the user running the parent process (modulo some safeguards we have around UNC paths, I think), this can read a lot of potentially sensitive files.

The general options we have here to change the current situation are:

1. Do not expose symlinks ever. Skip them in directory enumeration, do not let the user pick them individually.
2. Do let the user pick them individually, but skip them in directory enumeration.
3. Only allow symlinks that point within the directory tree that was selected, but as noted, we don't provide the ability to expose metadata about it being a symlink, so this is not particularly useful.
4. Do not expose the contents of the targets of symlinks but display something useless like "this is a symlink to /PATH", which potentially still would include private data.
5. Add a bunch of user prompting that will be hard to explain and where the development is unlikely to happen because it involves touching a lot of things.

:smaug, what are your current views on supporting symlinks given the threat possibilities here?
:dveditz, do we have an existing set of threat models about symlinks and user intent versus social engineering, etc.?

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

There is also option 6, to keep the current behavior. That is probably least surprising to the users (browser's file picking working like it works in other applications).

1 or 2 maybe. But those are data loss issues. User trying to select a directory, but not all its content is selected after all.
But I guess it could be 1.

(I don't understand what 3 means)

Comment 17

[Jens Stutte [:jstutte]]

FWIW chrome opted for 2, IIUC. So if the user clicks explicitly on a single file, it will always be uploaded, but for folders symlinks are always ignored, be they pointing to folders or files. I assume (but did not check) that this implies to follow the symlink if the picked folder itself is one. I can understand that as a compromise that respects the explicit choice of the user but avoids implicit surprises.

I read 3 as: Like 2 but only if the symlink points somewhere outside the selected folder. In terms of uploaded data this would actually double the payload for symlinks that point inside our folder, I fear, so probably not really wanted.

4 is a bit like the default behavior of tar on a folder that contains symlinks (though zip follows them by default, I think). Hard to say how surprising this would be to a normal linux user - at least it would give the chance to see what happened a posteriori in the uploaded data and not just silently omit files. In terms of uploaded file content this would be equivalent to 2, we just add the information of there being symlinks.

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox111 (beta) and tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in 6 days. However, the bug still isn't assigned.

Comment 19

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #16)

There is also option 6, to keep the current behavior. That is probably least surprising to the users (browser's file picking working like it works in other applications).

(Note: I recognize you are not advocating for this option, just filling out the list of options; I did not include the status quo option because I was only listing changes, but I agree that it's an option, just a bad one.)

Least surprising I think presumes all users know about symlinks and actively include them in their mental model. I think if we did a user research study, most users would be surprised that the following sequence would allow an attacker to be able to read arbitrary files of their choosing outside of the directory the user selected to upload.

1. Download a file archive.
2. Extract the file archive.
3. Select the directory for upload via file picker.

1 or 2 maybe. But those are data loss issues. User trying to select a directory, but not all its content is selected after all.
But I guess it could be 1.

You're right that the above set of steps is data-lossy versus the user re-archiving the contents of the directory. But the reality is that the File API does not expose the full set of file metadata exposed by the operating system, just the MIME type. (It is also the case that FileSystemEntry does not expose anything that can convey symlink-ness.) We could of course attempt to convey the symlink nature via the MIME type, but the IANA media type assignments list doesn't seem to actually have a type for symlinks that I can see.

I should note that File System Access WICG proposal which we explicitly do not implement also does not have any mechanism for exposing symlink metadata, although section 5.1: Users giving access to more, or more sensitive files than they intended does explicitly make mention of the potential magic of ".lnk" files on windows is interesting.

(I don't understand what 3 means)

Jens parsed this consistent with what I was trying to convey; we could distinguish between symlinks pointing within the directory sub-tree that the user has selected from those that point outside it. But as Jens points out, sending a second copy of data if we follow the symlink is not particularly useful. And if we expose the path of the symlink, there is some additional potential for revealing information that the user might not have intended to reveal; like if the symlink was created by the user, exposing the path may expose information about the layout of their filesystem which could be useful for subsequent attacks via different mechanisms.

I briefly looked at git/github for prior art:

• git explicitly has a concept of symbolic links; the index format has a specific object type encoding for "sybolic link"; there is a 9-bit unix permission field, but 0 is explicitly used for symlinks (and only 0755 and 0644 are valid otherwise).
• github makes a point to not serve as a CDN, so we can't see what MIME type it might use, but interestingly their API does have specific behavior for symlinks of resolving links within the repo and serving up the path if it's not inside the repo. The threat model of that case is of course extremely different, although it's worth noting that people regularly mess up and commit private/privileged data to git they did not intend to commit!

Comment 20

[Daniel Veditz [:dveditz]]

Option 2 from comment 15 is safe enough and most consistent with both Safari and Chrome. Both browsers allow the initial pick to be a symlink to somewhere else, but silently ignore any symlink found within it. In terms of standard unix utility behavior

• Option 1 is like find -P DIR -type f
• Option 2 is like find -H DIR -type f (Safari's current behavior; my Chrome isn't recursing at all as if it had a -maxdepth 1 term added)
• our current behavior is find -L DIR -type f

For the average user who has no idea what a symlink is this will avoid unexpected and unsafe surprises, while avoiding mysterious failures if they happen to pick a location that was symlinked elsewhere by their admin or a program they use. Power users who have symlinked files they'd like to upload will be able to figure out workarounds. It's a reasonable tradeoff, as Jens noted in comment 17

What about single file uploads? You could try to argue similarly that bad things still happen if you can convince a user to upload a specific one of the booby-trapped symlinks from the original expanded archive. I really can't buy it. Why would someone re-upload a file they just downloaded that they've never opened? I didn't find the original "upload a just-downloaded directory" attack all that plausible, but maybe there's a chance it could happen. If there's a lot of files, and most were as expected, users probably didn't check each one. With a single file I just can't believe a user is going to re-upload without checking. If they naive enough to do that I can't believe they would have anything at a predictable location that's worth stealing (for example, ssh keys, AWS or other cloud tokens, etc). They'd have a Firefox profile full of cookies and such, but this kind of scenario is exactly why our profile directories have a good chunk of randomness in the name.

I really wish this feature had had a mature specification before everyone started implementing it in non-compatible ways.

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in 3 days. However, the bug still isn't assigned.

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in 2 days. However, the bug still isn't assigned.

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is in a day. However, the bug still isn't assigned.

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

This is a reminder regarding comment #11!

The bug is marked as tracked for firefox112 (nightly). We have limited time to fix this, the soft freeze is today. However, the bug still isn't assigned.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Too late for 112 at this point with it going to RC on Monday. Let's aim for 113 instead.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Hi Andrew, do we have an ETA on this? We have 2 weeks until 113 goes to RC.

Comment 27

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #26)

Hi Andrew, do we have an ETA on this? We have 2 weeks until 113 goes to RC.

This is probably 3-4 weeks out at this point due to its relative priority and an imminent team meet-up.

Comment 29

[Daniel Veditz [:dveditz]]

The chrome bugs are public and we're picking up duplicates. No point to keep hidden.

Comment 30

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Bug 1813299 - Filter out symlinks for webkitdirectory. r=smaug!

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0eb72829d870
Filter out symlinks for webkitdirectory. r=smaug

Comment 32

[Iulian Moraru]

Backed out for causing failures on test_basic.html.

• Push with failures

• Failure log

• Backout link

[task 2023-05-31T16:08:20.673Z] 16:08:20     INFO -  TEST-START | dom/filesystem/tests/test_basic.html
[task 2023-05-31T16:14:43.745Z] 16:14:43     INFO -  wait for org.mozilla.geckoview.test_runner complete; top activity=org.mozilla.geckoview.test_runner
[task 2023-05-31T16:14:43.745Z] 16:14:43     INFO -  org.mozilla.geckoview.test_runner unexpectedly found running. Killing...
[task 2023-05-31T16:14:56.960Z] 16:14:56  WARNING -  TEST-UNEXPECTED-FAIL | dom/filesystem/tests/test_basic.html | application timed out after 370 seconds with no output
[task 2023-05-31T16:14:56.960Z] 16:14:56     INFO -  runtestsremote.py | Application ran for: 0:06:40.886368
[task 2023-05-31T16:14:57.031Z] 16:14:57     INFO -  Stopping web server
[task 2023-05-31T16:14:57.034Z] 16:14:57     INFO -  Server shut down.
[task 2023-05-31T16:14:57.054Z] 16:14:57     INFO -  Web server killed.
[task 2023-05-31T16:14:57.055Z] 16:14:57     INFO -  Stopping web socket server
[task 2023-05-31T16:14:57.075Z] 16:14:57     INFO -  Stopping ssltunnel
[task 2023-05-31T16:14:57.095Z] 16:14:57     INFO -  runtests.py | Running tests: end.
[task 2023-05-31T16:14:57.289Z] 16:14:57     INFO -  Buffered messages finished
[task 2023-05-31T16:14:57.289Z] 16:14:57     INFO -  Running manifest: dom/manifest/test/mochitest.ini
[task 2023-05-31T16:14:57.491Z] 16:14:57     INFO -  runtests.py | Failed to copy /builds/worker/workspace/build/tests/mochitest/hyphenation to profile
[task 2023-05-31T16:14:57.613Z] 16:14:57     INFO -  PID 3247 | pk12util: PKCS12 IMPORT SUCCESSFUL
[task 2023-05-31T16:14:57.675Z] 16:14:57     INFO -  MochitestServer : launching ['/builds/worker/workspace/build/hostutils/host-utils-108.0a1.en-US.linux-x86_64/xpcshell', '-g', '/builds/worker/workspace/build/hostutils/host-utils-108.0a1.en-US.linux-x86_64', '-f', '/builds/worker/workspace/build/hostutils/host-utils-108.0a1.en-US.linux-x86_64/components/httpd.js', '-e', "const _PROFILE_PATH = '/tmp/tmp3_3oyspp.mozrunner'; const _SERVER_PORT = '8854'; const _SERVER_ADDR = '10.0.2.2'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', '/builds/worker/workspace/build/tests/mochitest/server.js']
[task 2023-05-31T16:14:57.675Z] 16:14:57     INFO -  runtests.py | Server pid: 3256
[task 2023-05-31T16:14:57.678Z] 16:14:57     INFO -  runtests.py | Websocket server pid: 3259
[task 2023-05-31T16:14:57.682Z] 16:14:57     INFO -  runtests.py | SSL tunnel pid: 3263
[task 2023-05-31T16:14:57.733Z] 16:14:57     INFO -  use http3 server: 0
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running with scheme: http
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running with e10s: True
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running with fission: True
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running with cross-origin iframes: False
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running with serviceworker_e10s: True
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running with socketprocess_e10s: False
[task 2023-05-31T16:14:57.913Z] 16:14:57     INFO -  runtests.py | Running tests: start.
[task 2023-05-31T16:14:57.955Z] 16:14:57     INFO -  adb Granting important runtime permissions to org.mozilla.geckoview.test_runner
[task 2023-05-31T16:14:59.163Z] 16:14:59     INFO -  adb launch_application: am start -W -n org.mozilla.geckoview.test_runner/org.mozilla.geckoview.test_runner.TestRunnerActivity -a android.intent.action.MAIN --es env0 MOZ_CRASHREPORTER_NO_REPORT=1 --es env1 MOZ_CRASHREPORTER=1 --es env2 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env3 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env4 MOZ_IN_AUTOMATION=1 --es env5 R_LOG_LEVEL=6 --es env6 R_LOG_DESTINATION=stderr --es env7 R_LOG_VERBOSE=1 --es env8 XPCOM_DEBUG_BREAK=stack --es env9 MOZ_UPLOAD_DIR=/data/local/tmp/test_root/mozlog --es env10 MOZ_HIDE_RESULTS_TABLE=1 --es arg0 -no-remote --es arg1 -profile --es arg2 /data/local/tmp/test_root/profile/ --ez use_multiprocess True -d 'http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftmp%2Ftest_root%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO&hideResultsTable=1&manifestFile=tests.json&dumpOutputDirectory=%2Fdata%2Flocal%2Ftmp%2Ftest_root&ignorePrefsFile=ignorePrefs.json'
[task 2023-05-31T16:15:00.287Z] 16:15:00     INFO -  runtestsremote.py | Application pid: 4952
[task 2023-05-31T16:15:02.472Z] 16:15:02     INFO -  TEST-START | dom/manifest/test/test_ImageObjectProcessor_purpose.html

Comment 33

[Andrew Sutherland [:asuth] (he/him)]

Ah, so the symlink test won't work on android, but the canary test I used xpcom/tests/unit/test_symlinks.js had misleading information about where it runs exposed on searchfox:

Searchfox says:

This test gets skipped with pattern: os == "win"

The actual content in the ini file has a fail-if line that searchfox does not expose, whoops!:

[test_symlinks.js]
# Creating a symlink requires admin or developer mode on Windows.
skip-if = os == "win"
# Bug 676998: test fails consistently on Android
fail-if = os == "android"

I filed bug 1836639 on the searchfox enhancement to address this oversight. For this bug I will be expanding the exclusion for where we try to create symlinks to also skip doing so on android since it's not particularly important to the coverage and is consistent with our other symlink coverage. (Although the choice of "fail-if" is a little weird since it still tries to run the test, but it expects it to fail.) I am also adding a see-also from this bug to bug 676998 associated with the fail-if above in case someone does something to address that bug and the changes there could be relevant to the testing in this bug.

I will re-needinfo myself to land this after the tree re-opens.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70255c96976d
Filter out symlinks for webkitdirectory. r=smaug

Comment 35

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/70255c96976d

Comment 36

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 37

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:asuth, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Comment 38

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9335575 [details]
Bug 1813299 - Filter out symlinks for webkitdirectory. r=smaug!

Beta/Release Uplift Approval Request

• User impact if declined: The concern with this bug is that a user who has been socially engineered to extract an archive on linux or OS X containing hostile symlinks and then to select a directory for upload via a form element/drag-and-drop could expose the contents of sensitive files on their hard disk that they did not think they were exposing with the actions they were taking.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change is limited to our handling of symlinks and makes our code do less in general in a straightforward way. This change makes us conform to Chrome/Blink and Safari/Webkit's behavior and so we're not particularly concerned about there being weird use-cases out there that would break. (And to be clear, we intentionally do not want to support any weird use-cases that break.)

That said, it's not a major problem for this to just ride the trains, but it could be nice for this to be on ESR for consistency reasons.

• String changes made/needed:
• Is Android affected?: Yes

Comment 39

[Donal Meehan [:dmeehan]]

Comment on attachment 9335575 [details]
Bug 1813299 - Filter out symlinks for webkitdirectory. r=smaug!

Approved for 115.0b3.

Comment 40

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/994292710c9d

Comment 41

[Ameen]

Hi Mates i hope you all doing well, is this planned to released in 115 (or) 116?

Kindly confirm.

Comment 42

[Donal Meehan [:dmeehan]]

(In reply to Ameen from comment #41)

Hi Mates i hope you all doing well, is this planned to released in 115 (or) 116?

Kindly confirm.

This landed during the Fx116 cycle but was uplifted and will be released with Fx115

Comment 43

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 44

[Ameen]

Hi Jason Kratzer My name is mentioned as Ameen Basha instead of Ameen Basha M K.

Previous CVE's of mine are addressed as Ameen Basha M K. Kindly change it.

Comment 45

[Jason Kratzer [:jkratzer]]

Certainly! Thank you for letting me know!

Comment 46

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt