From 628bce99c404e37bbebc40555b591ed101400671 Mon Sep 17 00:00:00 2001 From: kink Date: Sun, 7 Jan 2007 17:30:13 +0000 Subject: [PATCH] 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 --- ChangeLog | 1 + functions/compose.php | 46 +++++++++++++++++++++++++++++++++++ functions/mailbox_display.php | 15 ++++-------- plugins/spamcop/functions.php | 17 +++---------- plugins/spamcop/spamcop.php | 2 ++ src/compose.php | 45 ++++++++++++---------------------- src/read_body.php | 1 + src/right_main.php | 1 + src/search.php | 1 + 9 files changed, 75 insertions(+), 54 deletions(-) diff --git a/ChangeLog b/ChangeLog index 66bfd163..c605b579 100644 --- 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 068fac0b..f64e49df 100644 --- 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 280c1873..625d48a7 100644 --- 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 e21fbfec..687a5b6d 100644 --- 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 b72ee0cb..de4ce276 100644 --- 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 de1c2f01..3c03f9fe 100644 --- 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 10c372b3..7078c95e 100644 --- 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 196342a1..85fe06c0 100644 --- 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 0b1623bd..2ecbf1f6 100644 --- 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 */ -- 2.25.1