From 1f270d3ccac30fd2e794923ff9aa0e0f07f59772 Mon Sep 17 00:00:00 2001 From: kink Date: Mon, 16 Jul 2007 20:48:46 +0000 Subject: [PATCH 1/1] Use attachment_dir only at the point where we're actually reading from / writing to the files, do not carry it around in the object. This makes us safer in the event the object is somehow exposed to the outside world. I may be cleaning this up some more for devel. git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@12541 7612ce4b-ef26-0410-bec9-ea0150e637f0 --- ChangeLog | 4 ++-- class/deliver/Deliver.class.php | 8 ++++++-- class/mime/Message.class.php | 8 ++++++-- functions/compose.php | 4 ++-- functions/mailbox_display.php | 4 +++- src/compose.php | 24 ++++++++++++++++-------- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 28532427..015fcf1d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -205,8 +205,8 @@ Version 1.5.2 - SVN - Added ability to detect HTTP_X_FORWARDED_PROTO in get_location(), thanks to Daniel Watts. - Fix test for signout.php in the logged in check in init.php so it - cannot be circumvented by manipulating the URL. External plugins migh - rely on init.php guarranteeing that the user is logged in. + cannot be circumvented by manipulating the URL. External plugins might + rely on init.php guaranteeing that the user is logged in. Version 1.5.1 (branched on 2006-02-12) -------------------------------------- diff --git a/class/deliver/Deliver.class.php b/class/deliver/Deliver.class.php index 0907faaf..0d797710 100644 --- a/class/deliver/Deliver.class.php +++ b/class/deliver/Deliver.class.php @@ -151,8 +151,10 @@ class Deliver { } $last = $body_part; } elseif ($message->att_local_name) { + global $username, $attachment_dir; + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); $filename = $message->att_local_name; - $file = fopen ($filename, 'rb'); + $file = fopen ($hashed_attachment_dir . '/' . $filename, 'rb'); while ($body_part = fgets($file, 4096)) { // remove NUL characters $body_part = str_replace("\0",'',$body_part); @@ -176,8 +178,10 @@ class Deliver { $this->writeToStream($stream, $body_part); } } elseif ($message->att_local_name) { + global $username, $attachment_dir; + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); $filename = $message->att_local_name; - $file = fopen ($filename, 'rb'); + $file = fopen ($hashed_attachment_dir . '/' . $filename, 'rb'); while ($tmp = fread($file, 570)) { $body_part = chunk_split(base64_encode($tmp)); // Up to 4.3.10 chunk_split always appends a newline, diff --git a/class/mime/Message.class.php b/class/mime/Message.class.php index 8677c92b..87449abb 100644 --- a/class/mime/Message.class.php +++ b/class/mime/Message.class.php @@ -1106,8 +1106,12 @@ class Message { * @since 1.5.1 */ function purgeAttachments() { - if ($this->att_local_name && file_exists($this->att_local_name)) { - unlink($this->att_local_name); + if ($this->att_local_name) { + global $username, $attachment_dir; + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); + if ( file_exists($hashed_attachment_dir . '/' . $this->att_local_name) ) { + unlink($hashed_attachment_dir . '/' . $this->att_local_name); + } } // recursively delete attachments from entities contained in this object for ($i=0, $entCount=count($this->entities);$i< $entCount; ++$i) { diff --git a/functions/compose.php b/functions/compose.php index f64e49df..ae86552e 100644 --- a/functions/compose.php +++ b/functions/compose.php @@ -18,7 +18,7 @@ * This function makes sure it doesn't overwrite other attachments, * preventing collisions and race conditions. * - * @return filename + * @return filename of the tempfile only (not full path) * @since 1.5.2 */ function sq_get_attach_tempfile() @@ -49,7 +49,7 @@ function sq_get_attach_tempfile() // success! make sure it's not readable, close and return filename chmod($full_localfilename, 0600); fclose($fp); - return $full_localfilename; + return $localfilename; } } diff --git a/functions/mailbox_display.php b/functions/mailbox_display.php index b54905c8..2730e123 100644 --- a/functions/mailbox_display.php +++ b/functions/mailbox_display.php @@ -1538,8 +1538,10 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) { $body = implode('', $body_a); $body .= "\r\n"; + global $username, $attachment_dir; $filename = sq_get_attach_tempfile(); - $fp = fopen($filename, 'wb'); + $fullpath = getHashedDir($username, $attachment_dir) . '/' . $filename; + $fp = fopen($fullpath, 'wb'); fwrite ($fp, $body); fclose($fp); diff --git a/src/compose.php b/src/compose.php index e49b174b..5f09af6b 100644 --- a/src/compose.php +++ b/src/compose.php @@ -330,7 +330,7 @@ if (sqsession_is_registered('session_expired_post')) { } else { // these are the vars that we can set from the expired composed session $compo_var_list = array ( 'send_to', 'send_to_cc','body','startMessage', - 'passed_body','use_signature','signature','attachments','subject','newmail', + 'passed_body','use_signature','signature','subject','newmail', 'send_to_bcc', 'passed_id', 'mailbox', 'from_htmladdr_search', 'identity', 'draft_id', 'delete_draft', 'mailprio', 'edit_as_new', 'compose_messsages', 'composesession', 'request_mdn', 'request_dr'); @@ -992,7 +992,7 @@ function newMail ($mailbox='', $passed_id='', $passed_ent_id='', $action='', $se * @return object */ function getAttachments($message, &$composeMessage, $passed_id, $entities, $imapConnection) { - global $squirrelmail_language, $languages; + global $squirrelmail_language, $languages, $username, $attachment_dir; if (!count($message->entities) || ($message->type0 == 'message' && $message->type1 == 'rfc822')) { @@ -1021,6 +1021,8 @@ function getAttachments($message, &$composeMessage, $passed_id, $entities, $imap function_exists($languages[$squirrelmail_language]['XTRA_CODE'] . '_encode')) { $filename = call_user_func($languages[$squirrelmail_language]['XTRA_CODE'] . '_encode', $filename); } + + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); $localfilename = sq_get_attach_tempfile(); $message->att_local_name = $localfilename; @@ -1028,7 +1030,7 @@ function getAttachments($message, &$composeMessage, $passed_id, $entities, $imap $localfilename); /* Write Attachment to file */ - $fp = fopen ($localfilename, 'wb'); + $fp = fopen ($hashed_attachment_dir . '/' . $localfilename, 'wb'); mime_print_body_lines ($imapConnection, $passed_id, $message->entity_id, $message->header->encoding, $fp); fclose ($fp); } @@ -1059,8 +1061,10 @@ function getMessage_RFC822_Attachment($message, $composeMessage, $passed_id, array_pop($body_a); $body = implode('', $body_a) . "\r\n"; + global $username, $attachment_dir; + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); $localfilename = sq_get_attach_tempfile(); - $fp = fopen($localfilename, 'wb'); + $fp = fopen($hashed_attachment_dir . '/' . $localfilename, 'wb'); fwrite ($fp, $body); fclose($fp); $composeMessage->initAttachment('message/rfc822',$subject.'.msg', @@ -1280,6 +1284,8 @@ function showInputForm ($session, $values=false) { } $attach = array(); + global $username, $attachment_dir; + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); // composeMessage can be empty when coming from a restored session if (is_object($composeMessage) && $composeMessage->entities) { foreach ($composeMessage->entities as $key => $attachment) { @@ -1293,7 +1299,7 @@ function showInputForm ($session, $values=false) { $a['Key'] = $key; $a['FileName'] = $attached_filename; $a['ContentType'] = $type; - $a['Size'] = filesize($attached_file); + $a['Size'] = filesize($hashed_attachment_dir . '/' . $attached_file); $attach[$key] = $a; } } @@ -1403,19 +1409,21 @@ function checkInput ($show) { /* True if FAILURE */ function saveAttachedFiles($session) { - global $compose_messages; + global $compose_messages, $username, $attachment_dir; /* get out of here if no file was attached at all */ if (! is_uploaded_file($_FILES['attachfile']['tmp_name']) ) { return true; } + $hashed_attachment_dir = getHashedDir($username, $attachment_dir); $localfilename = sq_get_attach_tempfile(); + $fullpath = $hashed_attachment_dir . '/' . $localfilename; // m_u_f works better with restricted PHP installs (safe_mode, open_basedir), // if that doesn't work, try a simple rename. - if (!@move_uploaded_file($_FILES['attachfile']['tmp_name'],$localfilename)) { - if (!@rename($_FILES['attachfile']['tmp_name'], $localfilename)) { + if (!@move_uploaded_file($_FILES['attachfile']['tmp_name'],$fullpath)) { + if (!@rename($_FILES['attachfile']['tmp_name'], $fullpath)) { return true; } } -- 2.25.1