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.

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)
 --------------------------------------

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;
+}
+
+

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;
 }
+

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;
 }

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);

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');
 }

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

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 */

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
  */