Overview

Until D17504, the "Show Raw File (Left)" and "Show Raw File (Right)" options in Differential generated an internal cache in Files which used default permissions (usually, "All Users"). Instead, these files should use the same permissions as the associated revision, which might be narrower.

Additionally, these files generated as permanent files. This is not a security concern, but other similar caches use temporary files.

After D17504, these files will generate with narrow permissions ("No One", plus a binding to the underlying diff), and generate as temporary files. That change also destroys all existing caches. This closes the hole, but will also make it impossible to audit potential disclosure impact because it destroys the evidence. See below for guidance on auditing.

Attack Scenarios

This issue was reported to us via HackerOne by a researcher. We have no evidence that it has been exploited, but a potential active attack scenario would look like this:

• Alice, a reviewer with access to privileged changes, is the victim.
• Eve, a user with access to an account but no access to privileged changes, is the attacker.
• Eve instructs Alice to navigate to a privileged revision in Differential and choose "Show Raw File (Right)" on a file she wishes to learn the content of. Eve can't see the revision, but might trick Alice into doing through social engineering, like asking Alice "Hey, does 'Show Raw File' work for you on 'secret_information.txt' in D123?". Alice may not realize that Eve does not have permission to take this action.
• After Alice clicks the link to generate the cache, Eve locates the generated cache in the main Files list and examines the file contents.

A passive attack would look like this:

• Eve, a user with access to an account but no accessed to privileged changes, is the attacker.
• Eve exhaustively searches Files to find caches which were disclosed incidentally during normal use of "Show Raw File".

In both cases, Eve must already control an account. This issue only allows Eve to escalate from control of an account to access to information which that account does not have permission to access.

The other three similar mechanisms ("Download Raw Patch" in Differential and Diffusion, "Show Raw File" in Diffusion) already use the correct permission model and use temporary files. It is likely that the vulnerable code predates significant portions of the Files and permissions systems, and was just overlooked as these other systems upgraded and gained more powerful policy and permissions capabilities.

Auditing and Response

If you want to assess the potential impact of this disclosure, you can run this script to get a list of file caches that exist. Only files on this list could have been disclosed. Save it as list-cached-files.php in phabricator/, then run it like this:

phabricator/ $ php -f list-cached-files.php
F123
F234
...

Here is the script:

<?php

require_once 'scripts/__init_script__.php';

$table = new DifferentialChangeset();
$conn = $table->establishConnection('w');
$viewer = PhabricatorUser::getOmnipotentUser();

$iterator = new LiskRawMigrationIterator(
  $conn,
  $table->getTableName());

$keys = array(
  'raw:new:phid',
  'raw:old:phid',
);

foreach ($iterator as $changeset) {
  try {
    $metadata = phutil_json_decode($changeset['metadata']);

    $phids = array();
    foreach ($keys as $key) {
      if (isset($metadata[$key])) {
        $phids[] = $metadata[$key];
      }
    }

    foreach ($phids as $phid) {
      $file = id(new PhabricatorFileQuery())
        ->setViewer($viewer)
        ->withPHIDs(array($phid))
        ->executeOne();
      if ($file) {
        echo "F".$file->getID()."\n";
      }
    }
  } catch (Exception $ex) {
    // Ignore.
  }
}

You can use this script in conjunction with bin/files cat to show file content instead of file monograms:

phabricator/ $ php -f list-cached-files.php | xargs -n1 ./bin/files cat

Note that you must run this script before you upgrade to D17504, since D17504 will destroy all these files and make it impossible to audit them.

If additional tools would be helpful in auditing and assessing impact, let us know what you need.

(Phabricator doesn't retain a detailed record of file access outside of any access log written by log.access.path, so we can't do much to distinguish between potential disclosure and actual disclosure, although you may be able to correlate list-cached-files.php with any access logs you maintain.)