[dev] imp-2.2.4 tmpfile problems ?

[Jarno Huuskonen]

On Fri, May 18, Anil Madhavapeddy wrote:
> Jarno Huuskonen wrote:
> >
> > When user uploads an attachment file from the compose
> > message window first php creates a temporary file
> > (in upload_tmp_dir) usually the file will be something
> > like /tmp/phpXXXXXX. Now it's upto compose.php3 to copy
> > the file to safe place, and compose.php3 does this with
> > copy(safe_file(...), safe_file(...) . '.att'). The
> > problem is that the copy call is kind enough to follow
> > symlinks.
> 
> The underlying system call used here by PHP4 is mkstemp().  glibc only
> accepts six X entries for randomness, which is why there are six there.
(AFAIK glibc-2.1.3 accepts more than six 'X' (it just uses the last six :)

I guess there is no problem with the actual file upload with php4, but the
problem is in the imp copy call (compose.php3:L915):

copy (safe_file($file_upload), safe_file($file_upload) . '.att');
      ^^^^^This is OK(at least w/php4), but the second argument (destination)
is just the original temporary filename with .att added to the end.

Example: php creates a temporary filename: /tmp/php2nRC4r so the copy call
will be:
copy('/tmp/php2nRC4r', '/tmp/php2nRC4r.att').
And the copy call does not check if the destination file '/tmp/php2nRC4r.att'
exists / is a symlink.

(OK I checked the php-4.0.5 source and it looks like that in safe mode 
the copy call has some checks (but I still think that even with that there's 
still temp-race)). Hope you can prove me wrong ;-)

> > All you have do is create bunch of /tmp/phpXXXXXX.att
> > symlinks and keep uploading attachments and you'll
> > end up (over)writing some file with webserver permissions.
> 
> Hang on, this wont work.  mkstemp() will never overwrite a symlink - it
> will create a unique filename, or fail.

See the .att in the linkname (php creates temporary filenames phpXXXXXX and
imp adds the .att (and imp doesn't use php_tempname/mkstemp)).
So do enough ln -s /nobody/writable/file /tmp/phpXXXXXX.att and pray that
it's your lucky day ;-) (OK ... It'll take time but eventually you'll get it
right).

> On OpenBSD at least, I've patched PHP4 to use 10 X characters, which
> makes it practically impossible to exploit.
> 
> http://demo.horde.org/devel/horde/chora/checkout.php/ports/www/php4/patc
> hes/patch-main_php_open_temporary_file_c?rt=obsd&r=1.1
> 
> > I noticed from the archive that Jon Parise has submitted
> > a patch for mimetypes.lib, so it'll use tempnam. Like I
> > mentioned above php-3.0.18 tempnam is not safe to use
> > (if the directory is world writable) because it has a temp-race
> > problem.
> 
> All the temp problems are fixed (or will be, as soon as I commit the
> rest of the new MIME stuff) in 2.3.x, since Horde::getTempFile() will
> take care of securely creating a temporary file.

When is imp-2.3.x going to be stable ? I think quite a few sites still run
2.2.x w/php3 ...

Hmm, Would something like this work (for imp-2.2.4) (with more sanity checks):

--- horde-1.2.4/imp/compose.php3        Fri May 18 14:46:02 2001
+++ horde-1.2.4-jh/imp/compose.php3       Fri May 18 15:30:26 2001
@@ -905,14 +905,16 @@
                if ($file_upload_size > 0) {
                        $attachments_name[] = $file_upload_name;
                        $attachments_size[] = $file_upload_size;
-                       $attachments_file[] = safe_file($file_upload) . '.att';
-      
+                       $tmpfile_name = tempnam(tmpdir(), 'impat');
+      $attachments_file[] = $tmpfile_name;
+
                        if (isset ($file_upload_type)) {
                                $attachments_type[] = $file_upload_type;
                        } else {
                                $attachments_type[] = '';
                        }
-                       copy (safe_file($file_upload), safe_file($file_upload) . '.att');
+                       copy (safe_file($file_upload), $tmpfile_name);
+                       chmod($tmpfile_name, 0600);
                        $msg = $message;
                }


(with php-3.0.18 tempnam fixed to use mkstemp).


-Jarno

-- 
Jarno Huuskonen - System Administrator   |  Jarno.Huuskonen@uku.fi
University of Kuopio - Computer Center   |  Work:   +358 17 162822
PO BOX 1627, 70211 Kuopio, Finland       |  Mobile: +358 40 5388169