change the way we do chmodd'ing. I know, my implementation really sucked... good we have motivated community members who point this out. ;) Thanks to faw for providing a way better function and for discussing and also abiding to our needs. :) LEW21 should maybe credited too... he gave the inspiration without knowing it.

git-svn-id: file:///svn/phpbb/branches/phpBB-3_0_0@8780 89ea8834-ac86-4346-8a33-228a782c2dd0
This commit is contained in:
Meik Sievertsen 2008-08-22 12:52:48 +00:00
parent 88c324a2a3
commit 6c763cd8b6
14 changed files with 140 additions and 108 deletions

View file

@ -89,7 +89,7 @@
<li>[Fix] Correctly set topic starter if first post in topic removed (Bug #30575 - Patch by blueray2048)</li>
<li>[Change] No longer allow the direct use of MULTI_INSERT in sql_build_array. sql_multi_insert() must be used.</li>
<li>[Change] Display warning in ACP if config.php file is left writable.</li>
<li>[Change] More restrictive chmod to new files being created.</li>
<li>[Change] More restrictive chmod to new files being created. (phpbb_chmod() function mostly by faw)</li>
<li>[Feature] Allow limited inheritance for templates sets.</li>
<li>[Feature] Allow hard disabling of the template editor.</li>
<li>[Fix] Delete avatar files (Bug #29985).</li>

View file

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

View file

@ -1196,7 +1196,7 @@ class acp_attachments
if (!file_exists($phpbb_root_path . $upload_dir))
{
@mkdir($phpbb_root_path . $upload_dir, 0777);
phpbb_chmod($phpbb_root_path . $upload_dir, 'rwrite');
phpbb_chmod($phpbb_root_path . $upload_dir, CHMOD_READ | CHMOD_WRITE);
}
}

View file

@ -277,7 +277,7 @@ class acp_language
{
trigger_error("Could not create directory $dir", E_USER_ERROR);
}
phpbb_chmod($dir, 'write-all');
@chmod($dir, 0777);
}
}
}

View file

@ -176,6 +176,11 @@ define('REFERER_VALIDATE_NONE', 0);
define('REFERER_VALIDATE_HOST', 1);
define('REFERER_VALIDATE_PATH', 2);
// phpbb_chmod() permissions
define('CHMOD_ALL', 7);
define('CHMOD_READ', 4);
define('CHMOD_WRITE', 2);
define('CHMOD_EXECUTE', 1);
// Additional constants
define('VOTE_CONVERTED', 127);

View file

@ -139,7 +139,7 @@ class dbal
}
// Connection closed correctly. Set db_connect_id to false to prevent errors
if (($result = $this->_sql_close()))
if ($result = $this->_sql_close())
{
$this->db_connect_id = false;
}

View file

@ -460,102 +460,136 @@ function _hash_crypt_private($password, $setting, &$itoa64)
}
/**
* 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
* Global function for chmodding directories and files for internal use
* This function determines owner and group whom the file belongs to and user and group of PHP and then set safest possible file permissions.
* The function determines owner and group from common.php file and sets the same to the provided file.
* The function uses bit fields to build the permissions.
* The function sets the appropiate execute bit on directories.
*
* Supported modes are:
* Supported constants representing bit fields 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.
* CHMOD_ALL - all permissions (7)
* CHMOD_READ - read permission (4)
* CHMOD_WRITE - write permission (2)
* CHMOD_EXECUTE - execute permission (1)
*
* 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'.
* NOTE: The function uses POSIX extension and fileowner()/filegroup() functions. If any of them is disabled, this function tries to build proper permissions, by calling is_readable() and is_writable() functions.
*
* @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
* @param $perms Permissions to set
* @return true on success, otherwise false
*
* @author faw, phpBB Group
*/
function phpbb_chmod($filename, $mode = 'read')
function phpbb_chmod($filename, $perms = CHMOD_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
// Return if the file no longer exists.
if (!file_exists($filename))
{
return false;
}
// Add the execute bit if it is a directory
if (!function_exists('fileowner') || !function_exists('filegroup'))
{
$file_uid = $file_gid = false;
$common_php_owner = $common_php_group = false;
}
else
{
global $phpbb_root_path, $phpEx;
// Determine owner/group of common.php file and the filename we want to change here
$common_php_owner = fileowner($phpbb_root_path . 'common.' . $phpEx);
$common_php_group = filegroup($phpbb_root_path . 'common.' . $phpEx);
$file_uid = fileowner($filename);
$file_gid = filegroup($filename);
// Try to set the owner to the same common.php has
if ($common_php_owner !== $file_uid && $common_php_owner !== false && $file_uid !== false)
{
// Will most likely not work
if (@chown($filename, $common_php_owner));
{
$file_uid = fileowner($filename);
}
}
// Try to set the group to the same common.php has
if ($common_php_group !== $file_gid && $common_php_group !== false && $file_gid !== false)
{
if (@chgrp($filename, $common_php_group));
{
$file_gid = filegroup($filename);
}
}
}
// And the owner and the groups PHP is running under.
$php_uid = (function_exists('posix_getuid')) ? @posix_getuid() : false;
$php_gids = (function_exists('posix_getgroups')) ? @posix_getgroups() : false;
// Who is PHP?
if ($file_uid === false || $file_gid === false || $php_uid === false || $php_gids === false)
{
$php = null;
}
else if ($file_uid == $php_uid /* && $common_php_owner !== false && $common_php_owner === $file_uid*/)
{
$php = 'owner';
}
else if (in_array($file_gid, $php_gids))
{
$php = 'group';
}
else
{
$php = 'other';
}
// Owner always has read/write permission
$owner = CHMOD_READ | CHMOD_WRITE;
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);
}
$owner |= CHMOD_EXECUTE;
// Set mode
$result = @chmod($filename, $chmod);
// Check for is_writable
if ($mode == 'rwrite')
// Only add execute bit to the permission if the dir needs to be readable
if ($perms & CHMOD_READ)
{
// 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');
}
$perms |= CHMOD_EXECUTE;
}
}
// Check for is_readable
if ($mode == 'rread')
switch ($php)
{
if (!is_readable($filename))
{
$result = phpbb_chmod($filename, 'sread');
case null:
case 'owner':
$result = @chmod($filename, ($owner << 6) + (0 << 3) + (0 << 0));
if (!is_readable($filename))
if (!is_null($php) || (!is_readable($filename) && is_writable($filename)))
{
$result = phpbb_chmod($filename, 'read');
break;
}
case 'group':
$result = @chmod($filename, ($owner << 6) + ($perms << 3) + (0 << 0));
if (!is_null($php) || ((!($perms & CHMOD_READ) || is_readable($filename)) && (!($perms & CHMOD_WRITE) || is_writable($filename))))
{
break;
}
case 'other':
$result = @chmod($filename, ($owner << 6) + ($perms << 3) + ($perms << 0));
if (!is_null($php) || ((!($perms & CHMOD_READ) || is_readable($filename)) && (!($perms & CHMOD_WRITE) || is_writable($filename))))
{
break;
}
default:
return false;
break;
}
return $result;

View file

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

View file

@ -562,7 +562,7 @@ class queue
$fp = @fopen($this->cache_file . '.lock', 'wb');
fclose($fp);
phpbb_chmod($this->cache_file . '.lock', 'write-all');
@chmod($this->cache_file . '.lock', 0777);
include($this->cache_file);
@ -697,7 +697,7 @@ class queue
@flock($fp, LOCK_UN);
fclose($fp);
phpbb_chmod($this->cache_file, 'rwrite');
phpbb_chmod($this->cache_file, CHMOD_WRITE);
}
}
@ -738,7 +738,7 @@ class queue
@flock($fp, LOCK_UN);
fclose($fp);
phpbb_chmod($this->cache_file, 'rwrite');
phpbb_chmod($this->cache_file, CHMOD_WRITE);
}
}
}

View file

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

View file

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

View file

@ -263,11 +263,10 @@ 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 string $chmod Permission mask for chmodding the file after a successful move. The mode entered here reflects the mode of phpbb_chmod()
* @param string $chmod Permission mask for chmodding the file after a successful move. The mode entered here reflects the mode of {@inline phpbb_chmod()}
* @access public
* @see phpbb_chmod()
*/
function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = 'rwrite')
function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false)
{
global $user, $phpbb_root_path;
@ -276,6 +275,8 @@ class filespec
return false;
}
$chmod = ($chmod === false) ? CHMOD_READ | CHMOD_WRITE : $chmod;
// We need to trust the admin in specifying valid upload directories and an attacker not being able to overwrite it...
$this->destination_path = $phpbb_root_path . $destination;
@ -346,16 +347,8 @@ class filespec
break;
}
// 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
$this->filesize = (@filesize($this->destination_file)) ? @filesize($this->destination_file) : $this->filesize;

View file

@ -438,14 +438,13 @@ class install_install extends module
if (!file_exists($phpbb_root_path . $dir))
{
@mkdir($phpbb_root_path . $dir, 0777);
phpbb_chmod($phpbb_root_path . $dir, 'rwrite');
phpbb_chmod($phpbb_root_path . $dir, CHMOD_READ | CHMOD_WRITE);
}
// Now really check
if (file_exists($phpbb_root_path . $dir) && is_dir($phpbb_root_path . $dir))
{
// Make writeable only for apache user
phpbb_chmod($phpbb_root_path . $dir, 'rwrite');
phpbb_chmod($phpbb_root_path . $dir, CHMOD_READ | CHMOD_WRITE);
$exists = true;
}
@ -875,7 +874,7 @@ class install_install extends module
}
@fclose($fp);
phpbb_chmod($phpbb_root_path . 'cache/install_lock', 'write-all');
@chmod($phpbb_root_path . 'cache/install_lock', 0777);
$load_extensions = implode(',', $load_extensions);
@ -928,8 +927,8 @@ class install_install extends module
if ($written)
{
// Readable by apache user/group, not by any other means
phpbb_chmod($phpbb_root_path . 'config.' . $phpEx, 'rread');
// We may revert back to chmod() if we see problems with users not able to change their config.php file directly
phpbb_chmod($phpbb_root_path . 'config.' . $phpEx, CHMOD_READ);
}
}

View file

@ -62,6 +62,7 @@ if ($id)
require($phpbb_root_path . 'includes/cache.' . $phpEx);
require($phpbb_root_path . 'includes/db/' . $dbms . '.' . $phpEx);
require($phpbb_root_path . 'includes/constants.' . $phpEx);
require($phpbb_root_path . 'includes/functions.' . $phpEx);
$db = new $sql_db();
$cache = new cache();