Problem/Motivation
Drupal may be configured to allow untrusted users to upload files with arbitrary filenames, including filenames with path components. For instance, if Drupal is installed at /var/www/ and the uploaded file repository is at /var/www/sites/example.com/files/, then an uploaded filename of '../../../index.php' would point to the /var/www/index.php file.
To prevent untrusted users from overwriting such critical files, the current logic for the DrupalLocalStreamWrapper::getLocalPath() function works like this:
-
Set $directory to the realpath() of the files repository.
-
Set $path to the target file location.
-
Set $realpath to realpath($path).
-
Return FALSE if $realpath does not begin with $directory.
This causes a problem when the site builder wants to store some uploaded files outside of the Drupal web root. Many site owners have resorted to hard-linked directories, recursive bind mounts, or simply hacking Drupal core to remove the restriction.
Proposed resolution
The logic within the DrupalLocalStreamWrapper::getLocalPath() function should be revised as follows:
-
Set $directory to the realpath() of the files repository.
-
Set $path to the target file location.
-
Set $realpath to realpath($path).
-
Return FALSE if both of the following are true:
$path contains '/..' or '\..'.$realpath does not begin with $directory
.
Remaining tasks
The MR symlinks_in_files_dir needs to be reviewed by the security team.
<!-- See https://drupal.org/core-mentoring/novice-tasks for tips on identifying novice tasks. Delete or add "Novice" from the Novice? column in the table below as appropriate. Uncomment tasks as the issue advances. Update the Complete? column to indicate when they are done, and maybe reference the comment number where they were done. -->
User interface changes
None.
API changes
The DrupalLocalStreamWrapper::getLocalPath() function would follow symlinks, even if they point outside the files repository.
Original report by tekante
Related issue:
#155781: "realpath" check breaks symbolic links in file directory →
The method by which directory ascension attacks are prevented in stream_wrappers.inc prevents file system layouts which rely on symlinks within the files directory. Specifically, the use of realpath within the getLocalPath results in the check of whether the destination directory exists within the files directory to fail.
I will attach a patch which allows for the destination directory to exist outside of the configured files directory when it appears this is due to the presence of a symlink and not due to directory ascension characters.