[drupal-devel] [bug] Error with file api and open_basedir restriction

Thu May 5 20:41:24 UTC 2005

Issue status update for http://drupal.org/node/5961

 Project:      Drupal
 Version:      cvs
 Component:    node system
 Category:     bug reports
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  Jose A Reyero
 Updated by:   nedjo
 Status:       patch

It seems likely that the image module issue - with using file_copy()
directly - can be addressed by adding a file_check_upload() call in the
file_copy() function by changing

<?php
  // Process a file upload object.
  if (is_object($source)) {
    $file = $source;
?>

to 

<?php
  // Process a file upload object.
  if (is_object($source)) {
    $file = file_check_upload($source);
?>

I can't immediately test this.  Do you want to, mpd?

nedjo

Previous comments:
------------------------------------------------------------------------

February 20, 2004 - 15:45 : Jose A Reyero

Attachment: http://drupal.org/files/issues/file-open-basedir.patch (1018 bytes)

I get this error when uploading a file through the file api, and the
upload fails:

warning: open_basedir restriction in effect. File is in wrong directory
in...

This happens when open_basedir restriction is activated and the
temporary directory is not in the allowed directories. There's a
related bug in the profile.module: http://drupal.org/node/view/2648 [1]

FIX:
The solution is to upload files using php's 'move_uploaded_file' -which
works even when open_basedir restriction is in effect.

This patch checks for the error conditions in
'file.inc::file_check_upload', and then uses move_uploaded_file to
upload the file to the temporary folder with a random name. Otherwise
-if the error conditions are not met-, does nothing.

RESTRICTIONS:
This patch introduces one additional restriction for the file api: Once
you have called 'file_check_upload', you have to use the returned object
when calling later 'file.api::file_save_upload'.
This shouldn't be a problem, as the right way to upload files should be
-I think, please correct me if I'm wrong-:

- $file=file_check_upload();
- (Perform validations with $file)
- file_save_upload($file,.....)

I will post also a very simple patch for profile.module to work with
this one. 

Anyway, as mentioned above, if the error conditions are not met, the
patch does nothing, so I think it should be applied first and then
check for other modules (profile.module) to use the file api the right
way and then be able to upload files when open_basedir restriction in
effect.

[1] http://drupal.org/node/view/2648

------------------------------------------------------------------------

November 28, 2004 - 13:38 : Goba

This seems to be an interesting alternate fix to my dump patch for
locale.module [2], which is the very same issue... Moving the file out
of the upload folder right away to the Drupal temp folder seems to be a
good idea to me. The only caveat is that files in the upload folder get
removed automatically, while one needs to remove the file from the
Drupal temp folder, once it is not needed anymore!
[2] http://drupal.org/node/13285

------------------------------------------------------------------------

December 8, 2004 - 15:26 : Goba

Please look at this stuff. We need to solve open_basedir problems at
least in core before the next point release. There are quite a few
users trying to use Drupal on shared hosting here, where open_basedir
is in effect.

------------------------------------------------------------------------

February 17, 2005 - 21:55 : clydefrog

This functionality gets a +1 from me, the user. We solved this problem
by changing upload_tmp_dir in the server config, but a fix in Drupal
would be more elegant.

------------------------------------------------------------------------

March 13, 2005 - 15:41 : killes at www.drop.org

Doesn't apply anymore.

------------------------------------------------------------------------

March 14, 2005 - 07:05 : Jose A Reyero

Attachment: http://drupal.org/files/issues/file-open-basedir_02.patch (810 bytes)

Just resubmitted the patch.

The problem is, from the time I sent the first patch , I've moved all
my sites, no shared webhosting anymore, so I don't have that upload
trouble anymore, and also cannot test it myself. That's also why I'm
unassigning this issue to me.

So please, somebody else test it.

------------------------------------------------------------------------

April 15, 2005 - 16:49 : mpd

Attachment: http://drupal.org/files/issues/file.inc-file_relocate.patch (3.87 KB)

I gave your patch a try.  It applies fine but doesn't address several
issues, particularily with the image module.

The attached patch makes attachments work, along with image (from cvs).
 I haven't seen any ill effects yet, but I believe that file_delete must
be used to avoid getting a build-up of dead files in the local temporary
directory (set to getcwd().'/tmp').  This isn't a problem with my
install, but it might be a problem with other modules.

In order for the patched code to work, $drupal/tmp must exist and be
writable by the web server.

------------------------------------------------------------------------

May 5, 2005 - 13:33 : nedjo

Attachment: http://drupal.org/files/issues/file_open_basedir_03.patch (817 bytes)

+1 for Jose's patch 

(I've attached an updated version, which I've tested successfully.  The
new version uses $file->filepath rather than $file->path and cleans up
the code a bit.)

While the larger patch suggested may have merit, I don't fully follow
it (not knowing what the image module issues are).  I'd like to see the
identified problem fixed first, and quickly.  Later cleanup and further
patches can follow.

This is a significant issue, meaning many in hosted environments can't
use Drupal's file uploading.  This has come up at least twice in the
help forums.  Jose's solution is simple.  Let's apply it!

This same changes work for 4.6.

------------------------------------------------------------------------

May 5, 2005 - 15:07 : mpd

I'll describe the issue that I found with file.inc concerning
image.module.

>From what I've seen, image.module calls file_copy() directly from
_image_insert().  file_copy() doesn't call file_check_upload(), so the
open_basedir problem remains because file_copy() attempts to access the
file while it's still in the restricted directory.  I'm sure that
there's a cleaner way to do it, but the patch that I submitted does the
trick for my purposes by addressing problems with both file_copy() and
file_check_upload().

I'll have a go at making a smaller patch that only addresses the issue
with file_copy(), since Nedjo's patch neatly fixes file_check_upload().