Posted by cburschka on June 30, 2007 at 8:10am
| Project: | ImageCache |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | active |
Issue Summary
It is impossible to place a symbolic link to an external directory into the file directory of a site, because file_check_location will run a realpath() check on it and determine that the "real" destination is not inside the files directory. This may prevent certain security issues, but I really don't see how blocking symbolic links (created by the site owner) is a good idea. There has to be a better way to do this...
I can't put this particular directory inside files physically, and I've had to hack out a large part of file_check_location in my site simply to make uploads work...
Comments
#1
i can confirm that this is a real problem and a real pain in the rumpus as of drupal v5.1 .
#2
it's not the file_check_location it's the file_create_path that checks if the file is in files directory.
#3
I'm not sure what you mean, but it's very demonstrably the change below that allows my modules to follow symbolic links in the file directory when creating new files.
function file_check_location($source, $directory = '') {- $check = realpath($source);
- if ($check) {
- $source = $check;
- }
- else {
- // This file does not yet exist
- $source = realpath(dirname($source)) .'/'. basename($source);
- }
- $directory = realpath($directory);
+ //$check = realpath($source);
+ //if ($check) {
+ // $source = $check;
+ //}
+ //else {
+ // // This file does not yet exist
+ // $source = realpath(dirname($source)) .'/'. basename($source);
+ //}
+ //$directory = realpath($directory);
if ($directory && strpos($source, $directory) !== 0) {
return 0;
}
Hack, yes; I needed this to work urgently. This removes the realpath call and effectively leaves the path as it is passed to the function. realpath replaces symbolic links with their actual locations, causing the path to fail the subsequent match, so commenting out the realpath check will circumvent this.
#4
I second that; we have a group of developers working on the same Drupal tree, and our files directory is large. Giving every developer their own copy of all the files is not an option for us.
As an aside, can someone explain to me what level of added security calling realpath() on the pathname provides? Why not check against the given string?
#5
realpath() resolves stuff like "dir1/dir2/../dir3" to "dir1/dir3". This is required to prevent people from using a relative path like ../../../../etc/passwd to break into stuff (though if Apache runs as root and /etc/passwd file is readable from the web, you're kind of asking for it). Unfortunately, as said, it also resolves symbolic links.
I would suggest something like
<?php$path = preg_replace('/\/+\.\/+/', '/', $path);
while (preg_match('/\/+[^\/\.]+\/+\.\.\/+/', $path)) $path = preg_replace('/\/+[^\/\.]+\/+\.\.\/+/', '/', $path);
?>
to take care of the relative path problem, and stay away from realpath entirely.
Edit: ../../../.. will not be replaced all at once, you need a while loop to resolve them until they're all gone. And I suppose there remains some problem with paths with periods. But I'm confident that a better regular expression can solve this.
#6
Addendum: The commenting-out of realpath as shown above is therefore a potential security hole. My site does not have public file uploads or a private file system, so there's no danger, but this may be dangerous elsewhere.
#7
This is still something of a hack and I'm still playing with it, but aims to be secure:
It is designed to work ONLY if the source is your_official_filedirectory/symbolic_link
Replace:
return 0;with
// One more chance that this source is ok:// see if source is in realpath of official_files_directory/symbolic_link
$sym_directory = realpath( $directory . "/" . basename($source));
if ($sym_directory && strpos($source, $sym_directory) !== 0) {
return 0; // is not just a symbolic link problem, return 0 ignoring all my code
}
#8
Subscribing
#9
I also face this problem when i use symlink images of my server, the imagecache module will not generate thumbnail image for a symlink image in the drupal's files directory.
I finally modify the function file_check_location to the following to make imagecache to work again.
function file_check_location($source, $directory = '') {
if(!is_link($source)){
$check = realpath($source);
if ($check) {
$source = $check;
}
else {
// This file does not yet exist
$source = realpath(dirname($source)) .'/'. basename($source);
}
$directory = realpath($directory);
if ($directory && strpos($source, $directory) !== 0) {
return 0;
}
}else{
global $_SERVER;
$directory = realpath($directory);
$source = $_SERVER['DOCUMENT_ROOT'] . '/'. $source;
if ($directory && strpos($source, $directory) !== 0) {
return 0;
}
}
return $source;
}
#10
We're also having to work around this at half a dozen locations -- the patches above didn't work properly for us in all instances so we've created another hybrid, if anyone is interested the patch and more information our situation is here: http://groups.drupal.org/node/18707#comment-64911.
I'm really interested in a more long-term solution as we've had to patch file.inc for the Open Media Project which means helping 7+ installs patch every time they upgrade core.....if someone has an idea for how we could change this or at least open it up to an admin setting ('enable symlinks') in file settings we'd be happy to put whatever time is necessary into developing the code and/or promoting it.
#11
still relevant in 6.9 . It'd be great if this was somehow resolved down the line such that a patch wasn't required.
#12
Man, this is a drag...
especially in cases where you have complex systems. Say I'm using image cache, and I've got an image server with thousands of images. I shouldn't have to copy an image to my drupal site to use it, a symlink to an nfs store with the originals shouldn't be an issue...
There must be a better way. I think the solution of just making sure there are no ~'s or .. in a path is good enough. How else could a user poke around outside of the files dir?
Best,
Jacob
#13
subscribe + bumping to the current version so it gets some attention.
#14
Aside from the fact that it doesn't work on windows...why not use mount --bind instead of symlink? Will that work?
#15
@greggles
It's possible to use mount, etc, but I think coming from the perspective of anyone with *nix experience, they will be pissed when they find some strange opaque bug which doesn't respect symlinks, something every other program in existence does...
The only exception I can think of is Apache, which will not (IIRC) follow symlinks by default, but can be set to do so. Perhaps this is a happy medium? I still think usability wise it is a bit ugly, but maybe we can make way to put the error and the resolution together in one message and that would help.
Best,
Jacob
#16
I think that if someone is comfortable creating symlinks they will be happy to make (or learn to make) a mount if we instruct them that's what they need. I agree on the happy medium of putting the error and resolution together in a message.
#17
This bug also breaks junction links in Windows, as far as I can tell, at least in 6.10.
#18
file_check_location no longer exists to cause a problem. This issue should be resolved at this juncture. The realpath is not used as a part of the URL generation anymore. You could also use bind mounts as a symlink work around.
#19
oh yeah, status.
#20
Automatically closed -- issue fixed for 2 weeks with no activity.
#21
This bug is biting me at the moment
I really want to keep the files directory as a symlink as this is nice and explicit as to where the files really are.
When I roll out a new release I keep the old version around for easy rollback - having files outside the document root with symlinks from each release to the files works really nicely.
I'll try and come up with a patch for this bug today.
#22
oops
I was looking at some old logs when I thought I hit this error and it turned out to be a config error that had already been fixed.
The site is working just fine with symlinks
#23
Automatically closed -- issue fixed for 2 weeks with no activity.
#24
Looks like this was closed in D7, then accidentally reopened under D6. I'm changing the version back to avoid confusion.
#25
subscribing
#26
Unless I am mistaken, this is still an open issue for imagecache on d6.
#27
Confirming #26, it is indeed still present as I just ran head first in to this issue. I'm not sure there's any way around this other than removing Imagecache's dependence on the file_create_path() call.
#28
I can confirm #15.
We had the same problem. We used one imagefolder throughout several sites. Mounting the folder allowed imagecache to work correctly again.
mount /path/src /path/dest --bind --ro
#29
I concur. The mount command does the trick. Although on my system (RHEL5 on x86_64) the syntax I used was:
mount --rbind /path/to/IMAGE_SERVER/sites/default/files /path/to/WEB_SERVER/sites/default/filesThis mounts everything in the files folder ("rbind" makes it recursive).
#30
I noticed that Drupal handles symlinked directories differently than physical directories referenced from the web root. Drupal will correctly intercept a "file not found" message internally and redirect to a custom error page with a file referenced from a physically located directory. However, if the file is referenced from a symlinked folder inside the web root, the .htaccess rewrite rule (ie. ErrorDocument 404 /index.php) will kick in and redirect to the home page. To demonstrate this, create a symlinked folder in the web root (ie. external) and make sure Apache is configured to follow symlinks (ie. Options +FollowSymLinks), then try to pull up a non-existent file in your browser (ie. http://www.example.com/external/notfound.png). You should get redirected to the home page. Now try referencing the same non-existent file in the sites folder (ie. http://www.example.com/sites/notfound.png). If you have a custom error page configured, it will redirect to that page. If anyone can explain this phenomenon, then I think we might be well on our way to a fix for imagecache. As mentioned earlier, this is a real drag.
#31
subscribing
#32
Okok, so the mount trick is supposed to solve the problem.
But plenty of people will still get a bleeding nose with this. Including me.
And,
http://www.seanodonnell.com/code/?id=74
Yet another setting to babysit.
What exactly is the purpose of the realpath() stuff in file_check_location() ?
What is the exploit they are talking about?
Otherwise, I'd suggest we get rid of file_create_path() for imagecache.
#33
Ok, looks like they mostly talk about exploits with '..'.
Whether symlinks are a risk - I don't think so.
I was able to rewrite file_create_path() as _imagecache_file_create_path(), but the result looks quite heavy and clunky, and I don't want to guarantee that it always works. (heck, we don't even have a spec or test cases)
What it does, roughly:
- replace all '/xyz/../' with '/'
- split all leading '../', and put into a dedicated variable.
- detect leading '/' (to give absolute paths a special treatment)
For paths that are not absolute and that don't have any leading '../' in them, the modified _imagecache_file_check_location() will do a string comparison, to check if the files dir is a prefix of the filepath to check.
Otherwise, it will do the usual realpath() stuff.
-----------
Note:
If we really wanted to check all theoretically possible symlink scenarios, we'd have to scan the entire files dir and subdirs for symlinks. This is not realistic, and generally not needed either.
We assume that the given filepath to check either can be checked with the string prefix comparison, or with the realpath() trick.
I might upload a patch later, but I am still ironing out the details (tomorrow is another day).
#34
Using
mount --bindrequires root access on the server, so it won't work for anyone on a shared web host.I'd agree that following symbolic links doesn't compromise any files on the webserver. If an unprivileged user can create an arbitrary symbolic link, that's already a vulnerability in itself.
So resolving links is an unwanted side effect of realpath(). Unfortunately, PHP doesn't seem to have anything that resolves ../ while leaving symbolic links intact. We may have to do that directly.
#35
I did work a bit on the logic, now I have a rewrite of file_check_location(), as _imagecache_file_check_location(), using recursion to allow symlinks.
This is not a patch yet, I rather want to discuss the logic.
<?php
function _imagecache_file_check_location($file, $supposed_parent) {
$supposed_parent_realpath = realpath($supposed_parent);
if (!$supposed_parent_realpath) {
// TODO: Decide on a reasonable behavior in this case.
return $file;
}
else {
$result = _imagecache_file_check_location_rec($file, $supposed_parent_realpath);
return $result;
}
}
function _imagecache_file_check_location_rec($file, $supposed_parent_realpath) {
$realpath = realpath($file);
if ($realpath && strpos($realpath, $supposed_parent_realpath) === 0) {
return $realpath;
}
else if ($file === '' || $file === '/' || $file === '.') {
// End of recursion.
return FALSE;
}
else {
$dirname = dirname($file);
$basename = basename($file);
if ($basename === '..') {
// Currently not supported, for the sake of an easier-to-read logic.
return FALSE;
}
else {
$rec = _imagecache_file_check_location_rec($dirname, $supposed_parent_realpath);
if ($rec) {
return $rec . '/' . $basename;
}
else {
return FALSE;
}
}
}
}
?>
#36
I've created a patch that allows for non-traversed paths and allows it. It's based on the patch for this similar issue in drupal 7 and 8 (see #1008402-35: Allow the use of symlinks within the files directory. )