file_save_upload() (as committed in #115267: Simplify File Uploads, Centralize File Validation and Quotas, Fix File Previews....) tries to apply the $validators before doing the following:

    // Move uploaded files from PHP's upload_tmp_dir to Drupal's temporary directory.
    // This overcomes open_basedir restrictions for future file operations.
    $file->filepath = $file->destination;
    if (!move_uploaded_file($_FILES['files']['tmp_name'][$source], $file->filepath)) {
      form_set_error($source, t('File upload error. Could not move uploaded file.'));
      watchdog('file', 'Upload error. Could not move uploaded file %file to destination %destination.', array('%file' =>  $file->filename, '%destination' => $file->filepath));
      return 0;
    }

The comment proves that an effort was made to consider the open_basedir restriction, but the authors/reviewers didn't realize that is_uploaded_file() and move_uploaded_file() (and inspecting $_FILES) is about all you can do as long as the file is in the /tmp directory.

The file needs to be moved before the $validators can be run on it.

For example, an image upload will fail with something like...

is_file(): open_basedir restriction in effect. File(/tmp/phpP9LR6m) is not within the allowed path(s): (/srv/www/apps/drupal:/srv/www/vhosts/example.com/httpdocs:/tmp/.drupal) in /srv/www/apps/drupal/drupal-6.8/includes/image.inc on line 117.

... (Drupal 6.8) if /tmp is not in the allowed path.

I found this in 6.8, but the code is unchanged in HEAD.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drewish’s picture

yeah i was discussing this over on #353580: file_validate_image_resolution doesn't allow not resizing if a toolkit is present.. we need to move it into our temp directory so we can validate/modify it and assuming that it passes validation then move it into the final destination.

drewish’s picture

Issue tags: +D7FileAPIWishlist
axyjo’s picture

subscribing.

axyjo’s picture

Status: Active » Needs work
FileSize
1.38 KB

Could it really be as simple as this? Am I omitting something significant?

drewish’s picture

axyjo, it's not that simple. did you read my comments in #1? we don't want to overwrite the file if it fails validation.

axyjo’s picture

I knew I was missing something. I wasn't quite sure what exactly. However, the issue states that we need to move it before the validators are run. Is there any place in Drupal that acts like /tmp and removes files from there periodically?

drewish’s picture

we could move it to our temp directory, run the validators, then move it to the final destination.

Andrew Schulman’s picture

subscribing

quicksketch’s picture

Subscribing.

nedjo’s picture

Status: Needs work » Needs review
FileSize
2.8 KB

Patch following drewish's suggestion. Seems to fix the error, but I hit this error only intermittently, making it hard to test.

nedjo’s picture

D6 version of the patch.

Status: Needs review » Needs work

The last submitted patch failed testing.

drewish’s picture

Status: Needs work » Needs review

the failures seem like they might have another cause. flipping the status to get a re-test.

salvis’s picture

Maybe there is an easier fix. Part of the issue is that the temporary directory set on admin/settings/file-system is ignored. For example, if you set it to /tmp/drupal, then you still get something like

is_file(): open_basedir restriction in effect. File(/tmp/phpiw2jDd) is not within the allowed path(s): (/srv/www/apps/drupal:/srv/www/vhosts/etg-bern.ch/httpdocs:/tmp/drupal)

Note that /tmp/drupal is in the allowed paths, but Drupal tried to create a file in /tmp rather than in /tmp/drupal!

Adding /tmp to the allowed paths is a work-around, but it shouldn't be necessary.

(This is for D6.)

Status: Needs review » Needs work

The last submitted patch failed testing.

danielbeeke2’s picture

For me this patch works (file_upload_357469_10_d6.patch), but now i get an other error

warning: move_uploaded_file() [function.move-uploaded-file]: open_basedir restriction in effect. File(/tmp/phpz1Xi1Q) is not within the allowed path(s): (/var/www/g30841/projectliberate.com) in /var/www/g30841/projectliberate.com/HTML/includes/file.inc on line 553.

When i submit the content i just made, everything works great

Daniel Beeke

imclean’s picture

salvis, any luck with this? Looks like you might be onto something, I can't see if/when the file is actually moved to Drupal's temp directory. danielbeeke, this could be the cause of your problem.

Or I should ask, is there any chance of this being fixed in D6?

dijuremo’s picture

We were having this exact problem and it all came down to a bad entry for the variable

upload_tmp_dir

in /etc/php.ini

After properly adjusting this entry and restarting apache, we were able to upload files properly. Note that based on your configuration, you may have that variable commented out in php.ini and are probably setting it elsewhere. If it is neither uncommented/defined in php.ini nor any other location, you will need to set it somewhere.

salvis’s picture

@dijuremo: Yes, you can make it work by massaging your environment to accommodate Drupal's quirks (and lose some security on the way), but the intent was and should still be to get Drupal to work correctly.

joosts’s picture

Hi all,

Any luck on this issue? I have run across the same issue (see http://drupal.org/node/1002048) and am now noticeing that I am not the only one.

Regards,
Joost

nedjo’s picture

Damien Tournoud’s picture

Status: Needs work » Closed (won't fix)

From #16 this seems like a won't fix anyway.

move_uploaded_file() is affected by open_basedir restrictions. That means that you have to make sure that the upload_tmp_dir is in the open_basedir paths for file uploads to work in any PHP application in the first place.

nedjo’s picture

Status: Closed (won't fix) » Needs work

Not convinced this is won't fix. On my limited testing the patch in #11 resolves the issue without raising the warning reported in #16. More research needed.

Byteme8199’s picture

Subscribed...

Caffeine Addict’s picture

I'd like to help get this one fixed, but don't know what needs fixing as the old patch above can't be applied anymore to the file.inc, any suggestions on where to start?

Here is the topic i started on this issue and am really interested in getting drupal working smoothly with multi hosting environments:
#1270862: ArchiverZip::extract() needs workaround for open_basedir & stream wrappers

On the bottom of the above topic i did some testing to do with uploading of modules with the open_base_dir restriction in place on the vhost for my drupal site.

salvis’s picture

any suggestions on where to start?

Check whether this issue is still present in D8. If yes, then we need a patch for D8.

Thank you for your help with this!

Caffeine Addict’s picture

So this won't get fixed on D7.X ?
Sure i'll download D8 and test now.

Caffeine Addict’s picture

Drupal 8 installs fine on the new version of zpanel with php 5.3.* and the open_base_dir setting enabled, but i need modules to install, plus i think the install module link is missing on the module administration page...

Unable to test...

Caffeine Addict’s picture

My issue is slightly different from this one, got directed from a different post to this one:
#1270862: ArchiverZip::extract() needs workaround for open_basedir & stream wrappers

Going to start some new tests for this bug, already done testing for the other bug on zip file extracts

salvis’s picture

Yes, it will (hopefully) be fixed in D7, and maybe even in D6, but all patches need to go to the latest version first and then be back-ported to older ones if necessary. We can't risk fixing something in D7 and leaving it broken in D8.

It's easier if you install a WAMP or LAMP server on your local computer. This is easier than it may appear — just try it.