Improve attachment file handling: use one new function to create a temp

file for storing the attachment. This replaces the same code in five
places. It also improves on the code, it's now much more safe against
overwriting existing attachments by chance.

git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@12097 7612ce4b-ef26-0410-bec9-ea0150e637f0

diff --git a/ChangeLog b/ChangeLog
index 66bfd163d62b80783bb8a82e141234ebb89c6231..c605b57975f93e38cf6018c7374594376d1d8802 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -171,6 +171,7 @@ Version 1.5.2 - CVS
   - Drop obsolete ORDB RBL from filters plugin (#1629398).
   - Add warning about magic_quotes_* in configtest.
   - Unify accepted versions for imap_server_type and set_defaults (#1629722).
+  - Improve attachment temp file creation.
 
 Version 1.5.1 (branched on 2006-02-12)
 --------------------------------------

diff --git a/functions/compose.php b/functions/compose.php
index 068fac0bd77d5eb607c46ca392a7d5e4c0c87582..f64e49dfef244c813b00d57dffd6f8deaba7f8a7 100644 (file)
--- a/functions/compose.php
+++ b/functions/compose.php
@@ -13,3 +13,49 @@
  */
 
 
+/**
+ * Get a new file to write an attachment to.
+ * This function makes sure it doesn't overwrite other attachments,
+ * preventing collisions and race conditions.
+ *
+ * @return filename
+ * @since 1.5.2
+ */
+function sq_get_attach_tempfile()
+{
+    global $username, $attachment_dir;
+
+    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
+
+    // using PHP >= 4.3.2 we can be truly atomic here
+    $filemods = check_php_version ( 4,3,2 ) ? 'x' : 'w';
+
+    // give up after 1000 tries
+    $TMP_MAX = 1000;
+    for ($try=0; $try<$TMP_MAX; ++$try) {
+
+        $localfilename = GenerateRandomString(32, '', 7);
+        $full_localfilename = "$hashed_attachment_dir/$localfilename";
+
+        // filename collision. try again
+        if ( file_exists($full_localfilename) ) {
+            continue;
+        }
+
+        // try to open for (binary) writing
+        $fp = @fopen( $full_localfilename, $filemods);
+
+        if ( $fp !== FALSE ) {
+            // success! make sure it's not readable, close and return filename
+            chmod($full_localfilename, 0600);
+            fclose($fp);
+            return $full_localfilename;
+        }
+    }
+
+    // we tried 1000 times but didn't succeed.
+    error_box( _("Could not open temporary file to store attachment. Contact your system administrator to resolve this issue.") );
+    return FALSE;
+}
+
+

diff --git a/functions/mailbox_display.php b/functions/mailbox_display.php
index 280c187392cd80ca18ff93dea296482e70bd3a14..625d48a7895353379b37b6513824ff08274b53d2 100644 (file)
--- a/functions/mailbox_display.php
+++ b/functions/mailbox_display.php
@@ -1477,9 +1477,6 @@ function handleMessageListForm($imapConnection,&$aMailbox,$sButton='',$aUid = ar
  * @author Marc Groot Koerkamp
  */
 function attachSelectedMessages($imapConnection,$aMsgHeaders) {
-    global $username, $attachment_dir,
-           $data_dir;
-
 
     sqgetGlobalVar('composesession', $composesession, SQ_SESSION);
     sqgetGlobalVar('compose_messages', $compose_messages, SQ_SESSION);
@@ -1496,8 +1493,6 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) {
         sqsession_register($composesession,'composesession');
     }
 
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
-
     $composeMessage = new Message();
     $rfc822_header = new Rfc822Header();
     $composeMessage->rfc822_header = $rfc822_header;
@@ -1517,14 +1512,13 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) {
             $body = implode('', $body_a);
             $body .= "\r\n";
 
-            $localfilename = GenerateRandomString(32, 'FILE', 7);
-            $full_localfilename = "$hashed_attachment_dir/$localfilename";
-
-            $fp = fopen( $full_localfilename, 'wb');
+            $filename = sq_get_attach_tempfile();
+            $fp = fopen($filename, 'wb');
             fwrite ($fp, $body);
             fclose($fp);
+
             $composeMessage->initAttachment('message/rfc822',$subject.'.msg',
-                 $full_localfilename);
+                 $filename);
         }
     }
 
@@ -1532,3 +1526,4 @@ function attachSelectedMessages($imapConnection,$aMsgHeaders) {
     sqsession_register($compose_messages,'compose_messages');
     return $composesession;
 }
+

diff --git a/plugins/spamcop/functions.php b/plugins/spamcop/functions.php
index e21fbfecf8f2b27bdc90261d0f9a8f13f8fae76a..687a5b6d86503a1b013123f4462b6b48d1513a98 100644 (file)
--- a/plugins/spamcop/functions.php
+++ b/plugins/spamcop/functions.php
@@ -180,9 +180,7 @@ function spamcop_enable_disable($option,$disable_action,$enable_action) {
  */
 function spamcop_getMessage_RFC822_Attachment($message, $composeMessage, $passed_id,
                                       $passed_ent_id='', $imapConnection) {
-    global $attachment_dir, $username;
 
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
     if (!$passed_ent_id) {
         $body_a = sqimap_run_command($imapConnection,
                                     'FETCH '.$passed_id.' RFC822',
@@ -198,21 +196,12 @@ function spamcop_getMessage_RFC822_Attachment($message, $composeMessage, $passed
         array_shift($body_a);
         $body = implode('', $body_a) . "\r\n";
 
-        $localfilename = GenerateRandomString(32, 'FILE', 7);
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-        $fp = fopen( $full_localfilename, 'w');
+        $filename = sq_get_attach_tempfile();
+        $fp = fopen($filename, 'wb');
         fwrite ($fp, $body);
         fclose($fp);
-
-        /* dirty relative dir fix */
-        if (substr($attachment_dir,0,3) == '../') {
-           $attachment_dir = substr($attachment_dir,3);
-           $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
-        }
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-
         $composeMessage->initAttachment('message/rfc822','email.txt',
-                         $full_localfilename);
+                         $filename);
     }
     return $composeMessage;
 }

diff --git a/plugins/spamcop/spamcop.php b/plugins/spamcop/spamcop.php
index b72ee0cbd745228ad73ec641061fb919024bca24..de4ce2760b0a28b5d67ca9abb801d43127b5fcc2 100644 (file)
--- a/plugins/spamcop/spamcop.php
+++ b/plugins/spamcop/spamcop.php
@@ -23,6 +23,8 @@ include_once(SM_PATH . 'functions/imap_messages.php');
 /* plugin functions */
 include_once(SM_PATH . 'plugins/spamcop/functions.php');
 
+include_once(SM_PATH . 'functions/compose.php');
+
 /* GLOBALS */
 
 sqgetGlobalVar('mailbox', $mailbox, SQ_GET);

diff --git a/src/compose.php b/src/compose.php
index de1c2f019ea07a6e3b4ace96769933660822cb14..3c03f9fea77920753c30762f0777744a342eb76c 100644 (file)
--- a/src/compose.php
+++ b/src/compose.php
@@ -26,6 +26,7 @@ require_once(SM_PATH . 'functions/imap_general.php');
 require_once(SM_PATH . 'functions/imap_messages.php');
 require_once(SM_PATH . 'functions/date.php');
 require_once(SM_PATH . 'functions/mime.php');
+require_once(SM_PATH . 'functions/compose.php');
 require_once(SM_PATH . 'class/deliver/Deliver.class.php');
 require_once(SM_PATH . 'functions/addressbook.php');
 require_once(SM_PATH . 'functions/forms.php');
@@ -974,8 +975,8 @@ function newMail ($mailbox='', $passed_id='', $passed_ent_id='', $action='', $se
  * @return object
  */
 function getAttachments($message, &$composeMessage, $passed_id, $entities, $imapConnection) {
-    global $attachment_dir, $username, $data_dir, $squirrelmail_language, $languages;
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
+    global $squirrelmail_language, $languages;
+
     if (!count($message->entities) ||
             ($message->type0 == 'message' && $message->type1 == 'rfc822')) {
         if ( !in_array($message->entity_id, $entities) && $message->entity_id) {
@@ -1003,19 +1004,14 @@ 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);
             }
-            $localfilename = GenerateRandomString(32, '', 7);
-            $full_localfilename = "$hashed_attachment_dir/$localfilename";
-            while (file_exists($full_localfilename)) {
-                $localfilename = GenerateRandomString(32, '', 7);
-                $full_localfilename = "$hashed_attachment_dir/$localfilename";
-            }
-            $message->att_local_name = $full_localfilename;
+            $localfilename = sq_get_attach_tempfile();
+            $message->att_local_name = $localfilename;
 
             $composeMessage->initAttachment($message->type0.'/'.$message->type1,$filename,
-                    $full_localfilename);
+                    $localfilename);
 
             /* Write Attachment to file */
-            $fp = fopen ("$hashed_attachment_dir/$localfilename", 'wb');
+            $fp = fopen ($localfilename, 'wb');
             mime_print_body_lines ($imapConnection, $passed_id, $message->entity_id, $message->header->encoding, $fp);
             fclose ($fp);
         }
@@ -1029,8 +1025,6 @@ function getAttachments($message, &$composeMessage, $passed_id, $entities, $imap
 
 function getMessage_RFC822_Attachment($message, $composeMessage, $passed_id,
         $passed_ent_id='', $imapConnection) {
-    global $attachment_dir, $username, $data_dir;
-    $hashed_attachment_dir = getHashedDir($username, $attachment_dir);
     if (!$passed_ent_id) {
         $body_a = sqimap_run_command($imapConnection,
                 'FETCH '.$passed_id.' RFC822',
@@ -1048,14 +1042,12 @@ function getMessage_RFC822_Attachment($message, $composeMessage, $passed_id,
         array_pop($body_a);
         $body = implode('', $body_a) . "\r\n";
 
-        $localfilename = GenerateRandomString(32, 'FILE', 7);
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-
-        $fp = fopen($full_localfilename, 'w');
+        $localfilename = sq_get_attach_tempfile();
+        $fp = fopen($localfilename, 'wb');
         fwrite ($fp, $body);
         fclose($fp);
         $composeMessage->initAttachment('message/rfc822',$subject.'.msg',
-                $full_localfilename);
+                $localfilename);
     }
     return $composeMessage;
 }
@@ -1381,33 +1373,26 @@ function checkInput ($show) {
 
 /* True if FAILURE */
 function saveAttachedFiles($session) {
-    global $_FILES, $attachment_dir, $username,
-        $data_dir, $compose_messages;
+    global $compose_messages;
 
     /* 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 = GenerateRandomString(32, '', 7);
-    $full_localfilename = "$hashed_attachment_dir/$localfilename";
-    while (file_exists($full_localfilename)) {
-        $localfilename = GenerateRandomString(32, '', 7);
-        $full_localfilename = "$hashed_attachment_dir/$localfilename";
-    }
+    $localfilename = sq_get_attach_tempfile();
 
     // 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'],$full_localfilename)) {
-        if (!@rename($_FILES['attachfile']['tmp_name'], $full_localfilename)) {
+    if (!@move_uploaded_file($_FILES['attachfile']['tmp_name'],$localfilename)) {
+        if (!@rename($_FILES['attachfile']['tmp_name'], $localfilename)) {
             return true;
         }
     }
     $message = $compose_messages[$session];
     $type = strtolower($_FILES['attachfile']['type']);
     $name = $_FILES['attachfile']['name'];
-    $message->initAttachment($type, $name, $full_localfilename);
+    $message->initAttachment($type, $name, $localfilename);
     $compose_messages[$session] = $message;
     sqsession_register($compose_messages , 'compose_messages');
 }

diff --git a/src/read_body.php b/src/read_body.php
index 10c372b3a104605b4367632fce0d09ca3d99af78..7078c95ef0035cedf731a965e6540234bbcfde93 100644 (file)
--- a/src/read_body.php
+++ b/src/read_body.php
@@ -27,6 +27,7 @@ require_once(SM_PATH . 'functions/identity.php');
 require_once(SM_PATH . 'functions/mailbox_display.php');
 require_once(SM_PATH . 'functions/forms.php');
 require_once(SM_PATH . 'functions/attachment_common.php');
+require_once(SM_PATH . 'functions/compose.php');
 
 /**
  * Given an IMAP message id number, this will look it up in the cached

diff --git a/src/right_main.php b/src/right_main.php
index 196342a19488cce18b57b2beeb0e55edd291b0ba..85fe06c0beceff23e82fcf044bcdd884cdcbf570 100644 (file)
--- a/src/right_main.php
+++ b/src/right_main.php
@@ -27,6 +27,7 @@ require_once(SM_PATH . 'functions/imap_messages.php');
 require_once(SM_PATH . 'functions/date.php');
 require_once(SM_PATH . 'functions/mime.php');
 require_once(SM_PATH . 'functions/mailbox_display.php');
+require_once(SM_PATH . 'functions/compose.php');
 
 
 /* lets get the global vars we may need */

diff --git a/src/search.php b/src/search.php
index 0b1623bd1dd6164a01ba427881c353307a7304f4..2ecbf1f68e2887378c9bdd45e8f21cd1c7c7b30e 100644 (file)
--- a/src/search.php
+++ b/src/search.php
@@ -31,6 +31,7 @@ require_once(SM_PATH . 'functions/mime.php');
 require_once(SM_PATH . 'functions/mailbox_display.php'); //getButton()
 require_once(SM_PATH . 'functions/forms.php');
 require_once(SM_PATH . 'functions/date.php');
+require_once(SM_PATH . 'functions/compose.php');
 
 /** Prefs array ordinals. Must match $recent_prefkeys and $saved_prefkeys
  */