Ticket #913 (new enhancement)

Opened 8 years ago

Last modified 8 years ago

Replace PHP move_uploaded_file() use in PEAR and TinyFCK

Reported by: jsteele Assigned to: jsteele
Priority: low Milestone:
Component: not categorised Severity: normal
Keywords: move_uploaded_file, copy, PEAR, TinyFCK, SimpleTest Cc:

Description

Although Seagull itself does not use this function, PEAR and TinyFCK do. The problem is that this function does not work the same as a copy()/unlink() due to possible temporary upload directory owner/group permissions differences.

An uploaded file using copy() will have these owner/group permissions:

(webserver user)/(webserver group) ex: nobody/users

One using move_uploaded_file() will have these:

(webserver user)/(directory group) ex: nobody/bin
php.ini upload_tmp_dir or default system $TMP/%TEMP%

The repercussions of this can be that the file can no longer be manipulated by the user (backup, mv, ftp, chmod, etc) if the owner is not in the same group as the temporary upload directory. An example would be /tmp directory with owner/group of bin/bin. Let's hope the webserver is not in the same group as /tmp.

This problem can also effect SimpleTest? usage. It *may* work the same as copy()/unlink() out-of-the-box depending on the system setup, but it *may not*, and that's just not portable.

I propose the following replacement (pseudo-code):

if(!is_uploaded_file($tmpfile))
  throw possible upload attack error
else if(!copy($tmpfile, $dest))
  throw copy failure with appropriate error code
else
  @unlink($tmpfile);

These are the current files affected:

lib/pear/HTML/QuickForm/file.php
lib/pear/PHP/data/func_array.php
www/tinyfck/filemanager/connectors/php/commands.php

I'm not sure how getting this replaced in these external packages works, but a single wrapper replacement function would be nice.

See the many notes at http://php.net/move_uploaded_file for varied other problems reported with this function.

Patch to follow...

Change History

04/26/06 17:30:17 changed by jsteele

  • owner changed from somebody to jsteele.
  • severity changed from not categorised to normal.
  • summary changed from Replace move_uploaded_file() to Replace PHP move_uploaded_file() use in PEAR and TinyFCK.

04/26/06 17:32:37 changed by jsteele

  • type changed from task to enhancement.