Page MenuHomePhabricator

Security Guidance: References to Files in Remarkup
Closed, ResolvedPublic

Description

The 2022 Week 21 Release of Phabricator includes important changes that resolve security issues with file permissions in Phabricator.

IMPORTANT: This release mitigates a severe security issue which allows attackers with few permission to gain access to files they can not otherwise see. All installs are strongly advised to upgrade.

Prior to these changes in this release, referencing a file in a remarkup block (like a comment, or a task description) with the {F678} syntax would attach the file to the object where the block appeared. For example, if you made a comment on task T123 which included the text {F678}, the file with file ID 678 would become attached to the task with ID 123.

When a file is attached to an object, any user who can view that object can view that file (a bit like attaching a file to an email), so this comment would allow anyone who can view T123 to also view F678. Almost always, this is desired and expected: for example, if you upload a screenshot to a task, you usually want users who can view the task to be able to view the screenshot.

Also note that this attachment occurs only if the acting user (usually, the user leaving the comment or making the edit) can already view the file: you can't attach files you can't view. That is, simply leaving a comment with {F1} {F2} ... does not enumerate and attach every file.

However, these rules have significant weaknesses. Assume Alice is an attacker who wants to view file F678, but does not have permission to view it. Bailey is a user who can view F678. Alice can potentially gain access to the file through these mechanisms:

  1. Alice creates a task with {F678} in the description, then asks Bailey to edit the task description trivially (for example, to fix a typo or add some information). When Bailey saves the new description, the existing {F678} reference activates with Bailey's permissions and attaches the file, giving Alice permission to view it. This behavior is almost certainly unexpected and surprising.
  2. Undisclosed Mechanism: A second, significantly more dangerous attack in this general vein also exists. The details of this attack will be disclosed at a later date, once installs have had some sort of plausible chance to upgrade. This weakness is much more severe than the first attack, and allows a user with a limited amount of initial access to gain widespread access to files without any social engineering or cooperation of a user with higher privileges.

Attack (1) was reported via HackerOne in https://hackerone.com/reports/763177 on Dec 22, 2020 (more than two years prior to this release). Attack (2) was reported via HackerOne in https://hackerone.com/reports/1560717 on May 5, 2022 (22 days prior to this release). These reports may not yet be public, depending on when you are reading this, so these links may not work. They should work within 60 days of this release.

After the changes in this release, simply referencing a file in remarkup is no longer sufficient to attach that file to an object. In the general case, files must now be attached explicitly. However, Phabricator can determine that an attachment is expected and safe in some common cases, like dragging a file into a comment area: when you do this, Phabricator will automatically generate an appropriate "attach file" action as part of the comment.

The new "Referenced Files" curtain element now shows files referenced on an object, including files that are referenced but not attached:

Screen Shot 2022-05-27 at 10.40.34 AM.png (618×318 px, 59 KB)

You can click the "File Not Attached" link to explicitly attach a file to an object:

Screen Shot 2022-05-27 at 10.41.26 AM.png (265×608 px, 28 KB)

The Files application now shows more details about object relationships, and allows you to detach a file. Detaching a file will remove the permissions granted by the attachment:

Screen Shot 2022-05-27 at 10.46.04 AM.png (518×955 px, 93 KB)

The visual design of these new elements aren't great. Some general file behaviors are also broken or rough; this release is aimed at plugging the security vulnerability. Some reasonable cases where Phabricator should be able to determine that an attachment is safe (e.g., using the "Upload File" dialog, and some unmodernized interfaces in various applicatinos) are not yet automatically identified as safe and must be manually attached. See T13682 for some discussion of future work. These behaviors may improve in a future release of Phabricator, but see also the "no longer actively maintained" banner and don't hold your breath.

If you encounter issues with this release, report them through the support portal of a Phacility instance that was registered prior to June 1, 2021. There is no other channel available to report bugs to the upstream. You may also consider migrating to software which is actively maintained.

Event Timeline

epriestley triaged this task as Normal priority.May 27 2022, 6:13 PM
epriestley created this task.

The details of this attack will be disclosed at a later date, once installs have had some sort of plausible chance to upgrade.

This attack was effectively disclosed in late July when the originating report was diclosed: https://hackerone.com/reports/1560717

A quick version of the attack was to push a commit like this:

Summary: steal every file hehehe <__< >__>

{F1}
{F2}
{F3}
...
{F999999}

Test Plan: yoink!

...to a writable repository you have permission to view. This would attach every file to the commit and grant you view permission on all of them. This requires the attacker have an account with commit access, but this is a low bar in many environments.

There are some remaining non-security bugs with this that I'll follow up on in T13682. I believe the security side of this is now resolved.