the chmod change i already had within the changelog (by mistake). This should further secure writable directories and created files.

Installation need to be tested on different hosts.

git-svn-id: file:///svn/phpbb/branches/phpBB-3_0_0@8763 89ea8834-ac86-4346-8a33-228a782c2dd0

phpBB/includes/acm/acm_file.php

@@ -93,7 +93,7 @@ class acm
 			@flock($fp, LOCK_UN);
 			fclose($fp);
 
-			@chmod($this->cache_dir . 'data_global.' . $phpEx, 0666);
+			phpbb_chmod($this->cache_dir . 'data_global.' . $phpEx, 'rwrite');
 		}
 		else
 		{
@@ -197,7 +197,7 @@ class acm
 				@flock($fp, LOCK_UN);
 				fclose($fp);
 
-				@chmod($this->cache_dir . "data{$var_name}.$phpEx", 0666);
+				phpbb_chmod($this->cache_dir . "data{$var_name}.$phpEx", 'rwrite');
 			}
 		}
 		else
@@ -416,7 +416,7 @@ class acm
 			@flock($fp, LOCK_UN);
 			fclose($fp);
 
-			@chmod($filename, 0666);
+			phpbb_chmod($filename, 'rwrite');
 
 			$query_result = $query_id;
 		}

phpBB/includes/acp/acp_attachments.php

@@ -279,7 +279,7 @@ class acp_attachments
 					{
 						$l_explain = (isset($user->lang[$vars['lang'] . '_EXPLAIN'])) ? $user->lang[$vars['lang'] . '_EXPLAIN'] : '';
 					}
-					
+
 					$content = build_cfg_template($type, $config_key, $this->new_config, $config_key, $vars);
 					if (empty($content))
 					{
@@ -1196,7 +1196,7 @@ class acp_attachments
 			if (!file_exists($phpbb_root_path . $upload_dir))
 			{
 				@mkdir($phpbb_root_path . $upload_dir, 0777);
-				@chmod($phpbb_root_path . $upload_dir, 0777);
+				phpbb_chmod($phpbb_root_path . $upload_dir, 'rwrite');
 			}
 		}
 

phpBB/includes/acp/acp_language.php

@@ -181,7 +181,7 @@ class acp_language
 			case 'submit_file':
 			case 'download_file':
 			case 'upload_data':
-				
+
 				if (!$submit || !check_form_key($form_name))
 				{
 					trigger_error($user->lang['FORM_INVALID']. adm_back_link($this->u_action), E_USER_WARNING);
@@ -261,23 +261,23 @@ class acp_language
 				if (!$safe_mode)
 				{
 					$mkdir_ary = array('language', 'language/' . $row['lang_iso']);
-					
+
 					if ($this->language_directory)
 					{
 						$mkdir_ary[] = 'language/' . $row['lang_iso'] . '/' . $this->language_directory;
 					}
-				
+
 					foreach ($mkdir_ary as $dir)
 					{
 						$dir = $phpbb_root_path . 'store/' . $dir;
-			
+
 						if (!is_dir($dir))
 						{
 							if (!@mkdir($dir, 0777))
 							{
 								trigger_error("Could not create directory $dir", E_USER_ERROR);
 							}
-							@chmod($dir, 0777);
+							phpbb_chmod($dir, 'write-all');
 						}
 					}
 				}
@@ -316,7 +316,7 @@ class acp_language
 							}
 
 							$entry = "\tarray(\n";
-							
+
 							foreach ($value as $_key => $_value)
 							{
 								$entry .= "\t\t" . (int) $_key . "\t=> '" . $this->prepare_lang_entry($_value) . "',\n";
@@ -433,7 +433,7 @@ class acp_language
 				{
 					trigger_error($user->lang['NO_LANG_ID'] . adm_back_link($this->u_action), E_USER_WARNING);
 				}
-				
+
 				$this->page_title = 'LANGUAGE_PACK_DETAILS';
 
 				$sql = 'SELECT *
@@ -442,7 +442,7 @@ class acp_language
 				$result = $db->sql_query($sql);
 				$lang_entries = $db->sql_fetchrow($result);
 				$db->sql_freeresult($result);
-				
+
 				$lang_iso = $lang_entries['lang_iso'];
 				$missing_vars = $missing_files = array();
 
@@ -488,7 +488,7 @@ class acp_language
 							trigger_error($user->lang['WRONG_LANGUAGE_FILE'] . adm_back_link($this->u_action . '&action=details&id=' . $lang_id), E_USER_WARNING);
 						}
 				}
-				
+
 				if (isset($_POST['remove_store']))
 				{
 					$store_filename = $this->get_filename($lang_iso, $this->language_directory, $this->language_file, true, true);
@@ -532,7 +532,7 @@ class acp_language
 						if (file_exists($phpbb_root_path . $this->get_filename($lang_iso, '', $file)))
 						{
 							$missing_vars[$file] = $this->compare_language_files($config['default_lang'], $lang_iso, '', $file);
-							
+
 							if (sizeof($missing_vars[$file]))
 							{
 								$is_missing_var = true;
@@ -550,7 +550,7 @@ class acp_language
 						if (file_exists($phpbb_root_path . $this->get_filename($lang_iso, 'acp', $file)))
 						{
 							$missing_vars['acp/' . $file] = $this->compare_language_files($config['default_lang'], $lang_iso, 'acp', $file);
-							
+
 							if (sizeof($missing_vars['acp/' . $file]))
 							{
 								$is_missing_var = true;
@@ -569,7 +569,7 @@ class acp_language
 							if (file_exists($phpbb_root_path . $this->get_filename($lang_iso, 'mods', $file)))
 							{
 								$missing_vars['mods/' . $file] = $this->compare_language_files($config['default_lang'], $lang_iso, 'mods', $file);
-								
+
 								if (sizeof($missing_vars['mods/' . $file]))
 								{
 									$is_missing_var = true;
@@ -581,7 +581,7 @@ class acp_language
 							}
 						}
 					}
-				
+
 					// More missing files... for example email templates?
 					foreach ($email_files as $file)
 					{
@@ -1046,7 +1046,7 @@ class acp_language
 				$compress->add_data('', 'language/' . $row['lang_iso'] . '/index.html');
 				$compress->add_data('', 'language/' . $row['lang_iso'] . '/email/index.html');
 				$compress->add_data('', 'language/' . $row['lang_iso'] . '/acp/index.html');
-				
+
 				if (sizeof($mod_files))
 				{
 					$compress->add_data('', 'language/' . $row['lang_iso'] . '/mods/index.html');
@@ -1208,7 +1208,7 @@ $lang = array_merge($lang, array(
 	function get_filename($lang_iso, $directory, $filename, $check_store = false, $only_return_filename = false)
 	{
 		global $phpbb_root_path, $safe_mode;
-		
+
 		$check_filename = "language/$lang_iso/" . (($directory) ? $directory . '/' : '') . $filename;
 
 		if ($check_store)

phpBB/includes/functions.php

@@ -459,6 +459,108 @@ function _hash_crypt_private($password, $setting, &$itoa64)
 	return $output;
 }
 
+/**
+* Global function for chmodding directories and files.
+* This function supports different modes to distinguish between writeable/non-writeable.
+* The function sets the appropiate execute bit on directories
+*
+* Supported modes are:
+*
+* rread (600):		Restrictive, only able to be read/write by the apache/site user.
+*					Used for files which only need to be accessible by phpBB itself and should never be accessible from the outside/web.
+* read (644):		Read-only permission for the site group/everyone. Used for ordinary files.
+* write (664):		Write-permission for the site group, read permission for everyone. Used for writeable files.
+* write-all (666):	Write-permission for everyone. Should only be used for temporary files.
+*
+* rwrite (0660):	Write-permission only for the site user/group. Used for files phpBB need to write to but within the cache/store/files directory.
+*
+* NOTE: If rwrite (restrictive write) is used, the function makes sure the file is writable by calling is_writable. If it is not, it falls back to 'write'
+* and then to 'write-all' to make sure the file is writable on every host setup.
+* NOTE: If rread (restrictive read) is used, the function makes sure the file is readable by calling is_readable. If it is not, it falls back to 'sread' (internal mode 640) and then to 'read'.
+*
+* @param $filename The file/directory to be chmodded
+* @param $mode The mode to set.
+* @return True on success, false if the mode was not set
+*/
+function phpbb_chmod($filename, $mode = 'read')
+{
+	switch ($mode)
+	{
+		case 'rread':
+			$chmod = 0600;
+		break;
+
+		// System-read, only used internally
+		case 'sread':
+			$chmod = 0640;
+		break;
+
+		case 'rwrite':
+			$chmod = 0660;
+		break;
+
+		case 'write':
+			$chmod = 0664;
+		break;
+
+		case 'write-all':
+			$chmod = 0666;
+		break;
+
+		case 'read':
+		default:
+			$chmod = 0644;
+		break;
+	}
+
+	// Return if the file no longer exist
+	if (!file_exists($filename))
+	{
+		return false;
+	}
+
+	// Add the execute bit if it is a directory
+	if (is_dir($filename))
+	{
+		// This line sets the correct execute bit on those "3-bits" being defined. 0644 becomes 0755 for example.
+		$chmod |= ($chmod & 7) ? 73 : (($chmod & 56) ? 72 : 64);
+	}
+
+	// Set mode
+	$result = @chmod($filename, $chmod);
+
+	// Check for is_writable
+	if ($mode == 'rwrite')
+	{
+		// We are in rwrite mode, so, make sure the file is writable
+		if (!is_writable($filename))
+		{
+			$result = phpbb_chmod($filename, 'write');
+
+			if (!is_writable($filename))
+			{
+				$result = phpbb_chmod($filename, 'write-all');
+			}
+		}
+	}
+
+	// Check for is_readable
+	if ($mode == 'rread')
+	{
+		if (!is_readable($filename))
+		{
+			$result = phpbb_chmod($filename, 'sread');
+
+			if (!is_readable($filename))
+			{
+				$result = phpbb_chmod($filename, 'read');
+			}
+		}
+	}
+
+	return $result;
+}
+
 // Compatibility functions
 
 if (!function_exists('array_combine'))

phpBB/includes/functions_compress.php

@@ -228,7 +228,7 @@ class compress_zip extends compress
 									{
 										trigger_error("Could not create directory $folder");
 									}
-									@chmod($str, 0777);
+									phpbb_chmod($str, 'rwrite');
 								}
 							}
 						}
@@ -257,7 +257,7 @@ class compress_zip extends compress
 								{
 									trigger_error("Could not create directory $folder");
 								}
-								@chmod($str, 0777);
+								phpbb_chmod($str, 'rwrite');
 							}
 						}
 					}
@@ -544,7 +544,7 @@ class compress_tar extends compress
 								{
 									trigger_error("Could not create directory $folder");
 								}
-								@chmod($str, 0777);
+								phpbb_chmod($str, 'rwrite');
 							}
 						}
 					}
@@ -571,7 +571,7 @@ class compress_tar extends compress
 							{
 								trigger_error("Could not create directory $folder");
 							}
-							@chmod($str, 0777);
+							phpbb_chmod($str, 'rwrite');
 						}
 					}
 
@@ -580,7 +580,7 @@ class compress_tar extends compress
 					{
 						trigger_error("Couldn't create file $filename");
 					}
-					@chmod($target_filename, 0777);
+					phpbb_chmod($target_filename, 'rwrite');
 
 					// Grab the file contents
 					fwrite($fp, ($filesize) ? $fzread($this->fp, ($filesize + 511) &~ 511) : '', $filesize);

phpBB/includes/functions_messenger.php

@@ -562,7 +562,7 @@ class queue
 
 		$fp = @fopen($this->cache_file . '.lock', 'wb');
 		fclose($fp);
-		@chmod($this->cache_file . '.lock', 0666);
+		phpbb_chmod($this->cache_file . '.lock', 'write-all');
 
 		include($this->cache_file);
 
@@ -683,7 +683,7 @@ class queue
 				break;
 			}
 		}
-	
+
 		if (!sizeof($this->queue_data))
 		{
 			@unlink($this->cache_file);
@@ -697,7 +697,7 @@ class queue
 				@flock($fp, LOCK_UN);
 				fclose($fp);
 
-				@chmod($this->cache_file, 0666);
+				phpbb_chmod($this->cache_file, 'rwrite');
 			}
 		}
 
@@ -713,11 +713,11 @@ class queue
 		{
 			return;
 		}
-		
+
 		if (file_exists($this->cache_file))
 		{
 			include($this->cache_file);
-			
+
 			foreach ($this->queue_data as $object => $data_ary)
 			{
 				if (isset($this->data[$object]) && sizeof($this->data[$object]))
@@ -738,7 +738,7 @@ class queue
 			@flock($fp, LOCK_UN);
 			fclose($fp);
 
-			@chmod($this->cache_file, 0666);
+			phpbb_chmod($this->cache_file, 'rwrite');
 		}
 	}
 }
@@ -1047,7 +1047,7 @@ class smtp_class
 			$err_msg .= $message;
 		}
 	}
-	
+
 	/**
 	* Log into server and get possible auth codes if neccessary
 	*/
@@ -1108,7 +1108,7 @@ class smtp_class
 				return false;
 			}
 
-			// If EHLO fails, we try HELO			
+			// If EHLO fails, we try HELO
 			$this->server_send("HELO {$local_host}");
 			if ($err_msg = $this->server_parse('250', __LINE__))
 			{
@@ -1129,7 +1129,7 @@ class smtp_class
 		{
 			return false;
 		}
-		
+
 		if (!isset($this->commands['AUTH']))
 		{
 			return (isset($user->lang['SMTP_NO_AUTH_SUPPORT'])) ? $user->lang['SMTP_NO_AUTH_SUPPORT'] : 'SMTP server does not support authentication';
@@ -1290,7 +1290,7 @@ class smtp_class
 		}
 
 		$md5_challenge = base64_decode($this->responses[0]);
-		
+
 		// Parse the md5 challenge - from AUTH_SASL (PEAR)
 		$tokens = array();
 		while (preg_match('/^([a-z-]+)=("[^"]+(?<!\\\)"|[^,]+)/i', $md5_challenge, $matches))

phpBB/includes/functions_posting.php

@@ -729,7 +729,7 @@ function create_thumbnail($source, $destination, $mimetype)
 		return false;
 	}
 
-	@chmod($destination, 0666);
+	phpbb_chmod($destination, 'rwrite');
 
 	return true;
 }

phpBB/includes/functions_template.php

@@ -755,7 +755,7 @@ class template_compile
 			@flock($fp, LOCK_UN);
 			@fclose($fp);
 
-			@chmod($filename, 0666);
+			phpbb_chmod($filename, 'rwrite');
 		}
 
 		return;

phpBB/includes/functions_upload.php

@@ -121,9 +121,9 @@ class filespec
 			case 'avatar':
 				$this->extension = strtolower($this->extension);
 				$this->realname = $prefix . $user_id . '.' . $this->extension;
-				
+
 			break;
-			
+
 			case 'unique_ext':
 			default:
 				$this->realname = $prefix . md5(unique_id()) . '.' . $this->extension;
@@ -228,8 +228,8 @@ class filespec
 	{
 		return @filesize($filename);
 	}
-	
+
-	
+
 	/**
 	* Check the first 256 bytes for forbidden content
 	*/
@@ -239,7 +239,7 @@ class filespec
 		{
 			return true;
 		}
-		
+
 		$fp = @fopen($this->filename, 'rb');
 
 		if ($fp !== false)
@@ -263,10 +263,11 @@ class filespec
 	*
 	* @param string $destination_path Destination path, for example $config['avatar_path']
 	* @param bool $overwrite If set to true, an already existing file will be overwritten
-	* @param octal $chmod Permission mask for chmodding the file after a successful move
+	* @param string $chmod Permission mask for chmodding the file after a successful move. The mode entered here reflects the mode of phpbb_chmod()
 	* @access public
+	* @see phpbb_chmod()
 	*/
-	function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = 0666)
+	function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = 'rwrite')
 	{
 		global $user, $phpbb_root_path;
 
@@ -345,7 +346,15 @@ class filespec
 				break;
 			}
 
-			@chmod($this->destination_file, $chmod);
+			// Backward compatibility - in versions prior to 3.0.3 $chmod was an octal
+			if (!is_string($chmod))
+			{
+				@chmod($this->destination_file, $chmod);
+			}
+			else
+			{
+				phpbb_chmod($this->destination_file, $chmod);
+			}
 		}
 
 		// Try to get real filesize from destination folder
@@ -416,7 +425,7 @@ class filespec
 		{
 			$size_lang = ($this->upload->max_filesize >= 1048576) ? $user->lang['MIB'] : (($this->upload->max_filesize >= 1024) ? $user->lang['KIB'] : $user->lang['BYTES'] );
 			$max_filesize = get_formatted_filesize($this->upload->max_filesize, false);
-	
+
 			$this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'WRONG_FILESIZE'], $max_filesize, $size_lang);
 
 			return false;
@@ -528,7 +537,7 @@ class fileupload
 			$this->max_filesize = (int) $max_filesize;
 		}
 	}
-	
+
 	/**
 	* Set disallowed strings
 	*/
@@ -872,7 +881,7 @@ class fileupload
 		{
 			$file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_EXTENSION'], $file->get('extension'));
 		}
-		
+
 		// MIME Sniffing
 		if (!$this->valid_content($file))
 		{

phpBB/install/install_install.php

@@ -438,16 +438,14 @@ class install_install extends module
 			if (!file_exists($phpbb_root_path . $dir))
 			{
 				@mkdir($phpbb_root_path . $dir, 0777);
-				@chmod($phpbb_root_path . $dir, 0777);
+				phpbb_chmod($phpbb_root_path . $dir, 'rwrite');
 			}
 
 			// Now really check
 			if (file_exists($phpbb_root_path . $dir) && is_dir($phpbb_root_path . $dir))
 			{
-				if (!@is_writable($phpbb_root_path . $dir))
+				// Make writeable only for apache user
-				{
+				phpbb_chmod($phpbb_root_path . $dir, 'rwrite');
-					@chmod($phpbb_root_path . $dir, 0777);
-				}
 				$exists = true;
 			}
 
@@ -877,7 +875,7 @@ class install_install extends module
 		}
 		@fclose($fp);
 
-		@chmod($phpbb_root_path . 'cache/install_lock', 0666);
+		phpbb_chmod($phpbb_root_path . 'cache/install_lock', 'write-all');
 
 		$load_extensions = implode(',', $load_extensions);
 
@@ -930,7 +928,8 @@ class install_install extends module
 
 			if ($written)
 			{
-				@chmod($phpbb_root_path . 'config.' . $phpEx, 0644);
+				// Readable by apache user/group, not by any other means
+				phpbb_chmod($phpbb_root_path . 'config.' . $phpEx, 'rread');
 			}
 		}