From 8174462e895966ce18c336b60dda93e562f4096f Mon Sep 17 00:00:00 2001 From: Meik Sievertsen Date: Fri, 22 Aug 2008 13:32:34 +0000 Subject: [PATCH] Merge chmod changes into trunk git-svn-id: file:///svn/phpbb/trunk@8781 89ea8834-ac86-4346-8a33-228a782c2dd0 --- phpBB/includes/acm/acm_file.php | 6 +- phpBB/includes/acp/acp_attachments.php | 2 +- phpBB/includes/constants.php | 5 + phpBB/includes/db/dbal.php | 2 +- phpBB/includes/functions.php | 134 +++++++++++++++++++++++++ phpBB/includes/functions_compress.php | 10 +- phpBB/includes/functions_messenger.php | 4 +- phpBB/includes/functions_posting.php | 2 +- phpBB/includes/functions_template.php | 2 +- phpBB/includes/functions_upload.php | 8 +- phpBB/install/install_install.php | 10 +- 11 files changed, 162 insertions(+), 23 deletions(-) diff --git a/phpBB/includes/acm/acm_file.php b/phpBB/includes/acm/acm_file.php index 1093ee1fa2..62d30f8018 100644 --- a/phpBB/includes/acm/acm_file.php +++ b/phpBB/includes/acm/acm_file.php @@ -84,7 +84,7 @@ class acm @flock($fp, LOCK_UN); fclose($fp); - @chmod($this->cache_dir . 'data_global.' . PHP_EXT, 0666); + phpbb_chmod($this->cache_dir . 'data_global.' . PHP_EXT, CHMOD_WRITE); } else { @@ -182,7 +182,7 @@ class acm @flock($fp, LOCK_UN); fclose($fp); - @chmod($this->cache_dir . "data{$var_name}." . PHP_EXT, 0666); + phpbb_chmod($this->cache_dir . "data{$var_name}." . PHP_EXT, CHMOD_WRITE); } } else @@ -392,7 +392,7 @@ class acm @flock($fp, LOCK_UN); fclose($fp); - @chmod($filename, 0666); + phpbb_chmod($filename, CHMOD_WRITE); $query_result = $query_id; } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index 8d6e91a71c..3b53d51417 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -1199,7 +1199,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, CHMOD_READ | CHMOD_WRITE); } } diff --git a/phpBB/includes/constants.php b/phpBB/includes/constants.php index 7c681a4040..01300c8992 100644 --- a/phpBB/includes/constants.php +++ b/phpBB/includes/constants.php @@ -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); diff --git a/phpBB/includes/db/dbal.php b/phpBB/includes/db/dbal.php index af5dfa059b..34a1261c2a 100644 --- a/phpBB/includes/db/dbal.php +++ b/phpBB/includes/db/dbal.php @@ -148,7 +148,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; } diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index c06c246082..e17bbfd080 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -484,6 +484,140 @@ function _hash_crypt_private($password, $setting, &$itoa64) } /** +* 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 constants representing bit fields are: +* +* CHMOD_ALL - all permissions (7) +* CHMOD_READ - read permission (4) +* CHMOD_WRITE - write permission (2) +* CHMOD_EXECUTE - execute permission (1) +* +* 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 $perms Permissions to set +* @return true on success, otherwise false +* +* @author faw, phpBB Group +*/ +function phpbb_chmod($filename, $perms = CHMOD_READ) +{ + // Return if the file no longer exists. + if (!file_exists($filename)) + { + return false; + } + + if (!function_exists('fileowner') || !function_exists('filegroup')) + { + $file_uid = $file_gid = false; + $common_php_owner = $common_php_group = false; + } + else + { + // Determine owner/group of common.php file and the filename we want to change here + $common_php_owner = fileowner(PHPBB_ROOT_PATH . 'common.' . PHP_EXT); + $common_php_group = filegroup(PHPBB_ROOT_PATH . 'common.' . PHP_EXT); + + $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)) + { + $owner |= CHMOD_EXECUTE; + + // Only add execute bit to the permission if the dir needs to be readable + if ($perms & CHMOD_READ) + { + $perms |= CHMOD_EXECUTE; + } + } + + switch ($php) + { + case null: + case 'owner': + $result = @chmod($filename, ($owner << 6) + (0 << 3) + (0 << 0)); + + if (!is_null($php) || (!is_readable($filename) && is_writable($filename))) + { + 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; +} + +/* * Checks if a path ($path) is absolute or relative * * @param string $path Path to check absoluteness of diff --git a/phpBB/includes/functions_compress.php b/phpBB/includes/functions_compress.php index 0e024f3a8d..eb25270738 100644 --- a/phpBB/includes/functions_compress.php +++ b/phpBB/includes/functions_compress.php @@ -226,7 +226,7 @@ class compress_zip extends compress { trigger_error("Could not create directory $folder"); } - @chmod($str, 0777); + phpbb_chmod($str, CHMOD_READ | CHMOD_WRITE); } } } @@ -255,7 +255,7 @@ class compress_zip extends compress { trigger_error("Could not create directory $folder"); } - @chmod($str, 0777); + phpbb_chmod($str, CHMOD_READ | CHMOD_WRITE); } } } @@ -540,7 +540,7 @@ class compress_tar extends compress { trigger_error("Could not create directory $folder"); } - @chmod($str, 0777); + phpbb_chmod($str, CHMOD_READ | CHMOD_WRITE); } } } @@ -567,7 +567,7 @@ class compress_tar extends compress { trigger_error("Could not create directory $folder"); } - @chmod($str, 0777); + phpbb_chmod($str, CHMOD_READ | CHMOD_WRITE); } } @@ -576,7 +576,7 @@ class compress_tar extends compress { trigger_error("Couldn't create file $filename"); } - @chmod($target_filename, 0777); + phpbb_chmod($target_filename, CHMOD_READ); // Grab the file contents fwrite($fp, ($filesize) ? $fzread($this->fp, ($filesize + 511) &~ 511) : '', $filesize); diff --git a/phpBB/includes/functions_messenger.php b/phpBB/includes/functions_messenger.php index 6ca894d0f2..0666275181 100644 --- a/phpBB/includes/functions_messenger.php +++ b/phpBB/includes/functions_messenger.php @@ -695,7 +695,7 @@ class queue @flock($fp, LOCK_UN); fclose($fp); - @chmod($this->cache_file, 0666); + phpbb_chmod($this->cache_file, CHMOD_WRITE); } } @@ -736,7 +736,7 @@ class queue @flock($fp, LOCK_UN); fclose($fp); - @chmod($this->cache_file, 0666); + phpbb_chmod($this->cache_file, CHMOD_WRITE); } } } diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index e7219a3f63..6e41249ff7 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -727,7 +727,7 @@ function create_thumbnail($source, $destination, $mimetype) return false; } - @chmod($destination, 0666); + phpbb_chmod($destination, CHMOD_READ | CHMOD_WRITE); return true; } diff --git a/phpBB/includes/functions_template.php b/phpBB/includes/functions_template.php index 94b676a77c..0cdd5d21a3 100644 --- a/phpBB/includes/functions_template.php +++ b/phpBB/includes/functions_template.php @@ -722,7 +722,7 @@ class template_compile @flock($destination_handle, LOCK_UN); @fclose($destination_handle); - @chmod($filename, 0666); + phpbb_chmod($filename, CHMOD_WRITE); return true; } diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index 4411f28e5a..00dc67438b 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -263,10 +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 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 {@inline phpbb_chmod()} * @access public */ - function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = 0666) + function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) { global $user; @@ -275,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; @@ -345,7 +347,7 @@ class filespec break; } - @chmod($this->destination_file, $chmod); + phpbb_chmod($this->destination_file, $chmod); } // Try to get real filesize from destination folder diff --git a/phpBB/install/install_install.php b/phpBB/install/install_install.php index 23e7c22dbb..a95cfbaae3 100644 --- a/phpBB/install/install_install.php +++ b/phpBB/install/install_install.php @@ -458,16 +458,13 @@ 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, CHMOD_READ | CHMOD_WRITE); } // Now really check if (file_exists(PHPBB_ROOT_PATH . $dir) && is_dir(PHPBB_ROOT_PATH . $dir)) { - if (!@is_writable(PHPBB_ROOT_PATH . $dir)) - { - @chmod(PHPBB_ROOT_PATH . $dir, 0777); - } + phpbb_chmod(PHPBB_ROOT_PATH . $dir, CHMOD_READ | CHMOD_WRITE); $exists = true; } @@ -952,7 +949,8 @@ class install_install extends module if ($written) { - @chmod(PHPBB_ROOT_PATH . 'config.' . PHP_EXT, 0644); + // 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.' . PHP_EXT, CHMOD_READ); } }