Subject: Fixed Bug #18056 [SECURITY]: Symlink attack in PEAR install [dufuz] Origin: http://svn.php.net/viewvc?view=revision&revision=308687 This feature is implemented as a result of the above fix * Implemented Request #16648: Use TMPDIR for builds instead of /var/tmp [dufuz] CVE-2011-1072 This patch differs from the upstream commit in that the packages.xml and packages2.xml edits containing only information about the commits were removed to reduce conflicts. Index: PEAR/PackageFile.php =================================================================== --- PEAR/PackageFile.php (revision 308686) +++ PEAR/PackageFile.php (revision 308687) @@ -46,11 +46,7 @@ */ var $_config; var $_debug; - /** - * Temp directory for uncompressing tgz files. - * @var string|false - */ - var $_tmpdir; + var $_logger = false; /** * @var boolean @@ -58,17 +54,23 @@ var $_rawReturn = false; /** + * helper for extracting Archive_Tar errors + * @var array + * @access private + */ + var $_extractErrors = array(); + + /** * * @param PEAR_Config $config * @param ? $debug * @param string @tmpdir Optional temporary directory for uncompressing * files */ - function PEAR_PackageFile(&$config, $debug = false, $tmpdir = false) + function PEAR_PackageFile(&$config, $debug = false) { $this->_config = $config; $this->_debug = $debug; - $this->_tmpdir = $tmpdir; } /** @@ -349,20 +351,17 @@ } } - if ($this->_tmpdir) { - $tmpdir = $this->_tmpdir; - } else { - $tmpdir = System::mkTemp(array('-t', $this->_config->get('temp_dir'), '-d', 'pear')); - if ($tmpdir === false) { - $ret = PEAR::raiseError("there was a problem with getting the configured temp directory"); - return $ret; - } - - PEAR_PackageFile::addTempFile($tmpdir); + $tmpdir = System::mktemp('-t ' . $this->_config->get('temp_dir') . ' -d pear'); + if ($tmpdir === false) { + $ret = PEAR::raiseError("there was a problem with getting the configured temp directory"); + return $ret; } + PEAR_PackageFile::addTempFile($tmpdir); + $this->_extractErrors(); PEAR::staticPushErrorHandling(PEAR_ERROR_CALLBACK, array($this, '_extractErrors')); + if (!$xml || !$tar->extractList(array($xml), $tmpdir)) { $extra = implode("\n", $this->_extractErrors()); if ($extra) { @@ -381,13 +380,6 @@ } /** - * helper for extracting Archive_Tar errors - * @var array - * @access private - */ - var $_extractErrors = array(); - - /** * helper callback for extracting Archive_Tar errors * * @param PEAR_Error|null $err Index: PEAR/Installer.php =================================================================== --- PEAR/Installer.php (revision 308686) +++ PEAR/Installer.php (revision 308687) @@ -1036,25 +1036,10 @@ // }}} // {{{ _parsePackageXml() - function _parsePackageXml(&$descfile, &$tmpdir) + function _parsePackageXml(&$descfile) { - if (substr($descfile, -4) == '.xml') { - $tmpdir = false; - } else { - // {{{ Decompress pack in tmp dir ------------------------------------- - - // To allow relative package file names - $descfile = realpath($descfile); - - if (PEAR::isError($tmpdir = System::mktemp('-d'))) { - return $tmpdir; - } - $this->log(3, '+ tmp dir created at ' . $tmpdir); - // }}} - } - // Parse xml file ----------------------------------------------- - $pkg = new PEAR_PackageFile($this->config, $this->debug, $tmpdir); + $pkg = new PEAR_PackageFile($this->config, $this->debug); PEAR::staticPushErrorHandling(PEAR_ERROR_RETURN); $p = &$pkg->fromAnyFile($descfile, PEAR_VALIDATE_INSTALLING); PEAR::staticPopErrorHandling(); @@ -1135,17 +1120,20 @@ $pkg = $pkgfile->getPackageFile(); $pkgfile = $pkg->getArchiveFile(); $descfile = $pkg->getPackageFile(); - $tmpdir = dirname($descfile); } else { $descfile = $pkgfile; - $tmpdir = ''; - $pkg = $this->_parsePackageXml($descfile, $tmpdir); + $pkg = $this->_parsePackageXml($descfile); if (PEAR::isError($pkg)) { return $pkg; } } + $tmpdir = dirname($descfile); if (realpath($descfile) != realpath($pkgfile)) { + // Use the temp_dir since $descfile can contain the download dir path + $tmpdir = $this->config->get('temp_dir', null, 'pear.php.net'); + $tmpdir = System::mktemp("-d -t $tmpdir"); + $tar = new Archive_Tar($pkgfile); if (!$tar->extract($tmpdir)) { return $this->raiseError("unable to unpack $pkgfile"); @@ -1373,9 +1361,8 @@ } } - $tmp_path = dirname($descfile); if (substr($pkgfile, -4) != '.xml') { - $tmp_path .= DIRECTORY_SEPARATOR . $pkgname . '-' . $pkg->getVersion(); + $tmpdir .= DIRECTORY_SEPARATOR . $pkgname . '-' . $pkg->getVersion(); } $this->configSet('default_channel', $channel); @@ -1401,9 +1388,9 @@ foreach ($filelist as $file => $atts) { $this->expectError(PEAR_INSTALLER_FAILED); if ($pkg->getPackagexmlVersion() == '1.0') { - $res = $this->_installFile($file, $atts, $tmp_path, $options); + $res = $this->_installFile($file, $atts, $tmpdir, $options); } else { - $res = $this->_installFile2($pkg, $file, $atts, $tmp_path, $options); + $res = $this->_installFile2($pkg, $file, $atts, $tmpdir, $options); } $this->popExpect(); Index: PEAR/Downloader.php =================================================================== --- PEAR/Downloader.php (revision 308686) +++ PEAR/Downloader.php (revision 308687) @@ -189,7 +189,8 @@ require_once 'System.php'; } - $tmp = System::mktemp(array('-d')); + $tmpdir = $this->config->get('temp_dir'); + $tmp = System::mktemp("-d -t $tmpdir"); $a = $this->downloadHttp('http://' . $channel . '/channel.xml', $this->ui, $tmp, $callback, false); PEAR::popErrorHandling(); if (PEAR::isError($a)) { @@ -493,14 +494,13 @@ */ function analyzeDependencies(&$params, $force = false) { - $hasfailed = $failed = false; if (isset($this->_options['downloadonly'])) { return; } PEAR::staticPushErrorHandling(PEAR_ERROR_RETURN); $redo = true; - $reset = false; + $reset = $hasfailed = $failed = false; while ($redo) { $redo = false; foreach ($params as $i => $param) { @@ -698,6 +698,7 @@ } } } + PEAR::staticPopErrorHandling(); if ($hasfailed && (isset($this->_options['ignore-errors']) || isset($this->_options['nodeps']))) { @@ -718,6 +719,7 @@ if (isset($this->_downloadDir)) { return $this->_downloadDir; } + $downloaddir = $this->config->get('download_dir'); if (empty($downloaddir) || (is_dir($downloaddir) && !is_writable($downloaddir))) { if (is_dir($downloaddir) && !is_writable($downloaddir)) { @@ -725,14 +727,17 @@ '" is not writeable. Change download_dir config variable to ' . 'a writeable dir to avoid this warning'); } + if (!class_exists('System')) { require_once 'System.php'; } + if (PEAR::isError($downloaddir = System::mktemp('-d'))) { return $downloaddir; } $this->log(3, '+ tmp dir created at ' . $downloaddir); } + if (!is_writable($downloaddir)) { if (PEAR::isError(System::mkdir(array('-p', $downloaddir))) || !is_writable($downloaddir)) { @@ -741,6 +746,7 @@ 'a writeable dir'); } } + return $this->_downloadDir = $downloaddir; } @@ -771,27 +777,11 @@ $this->_options = $options; } - // }}} - // {{{ setOptions() function getOptions() { return $this->_options; } - /** - * For simpler unit-testing - * @param PEAR_Config - * @param int - * @param string - */ - function &getPackagefileObject(&$c, $d, $t = false) - { - if (!class_exists('PEAR_PackageFile')) { - require_once 'PEAR/PackageFile.php'; - } - $a = &new PEAR_PackageFile($c, $d, $t); - return $a; - } /** * @param array output of {@link parsePackageName()} @@ -1000,9 +990,20 @@ } return $info; } elseif ($chan->supportsREST($this->config->get('preferred_mirror')) - && $base = $chan->getBaseURL('REST1.0', $this->config->get('preferred_mirror')) + && + ( + ($base2 = $chan->getBaseURL('REST1.3', $this->config->get('preferred_mirror'))) + || + ($base = $chan->getBaseURL('REST1.0', $this->config->get('preferred_mirror'))) + ) ) { - $rest = &$this->config->getREST('1.0', $this->_options); + if ($base2) { + $base = $base2; + $rest = &$this->config->getREST('1.3', $this->_options); + } else { + $rest = &$this->config->getREST('1.0', $this->_options); + } + $url = $rest->getDepDownloadURL($base, $xsdversion, $dep, $parr, $state, $version, $chan->getName()); if (PEAR::isError($url)) { @@ -1707,6 +1708,10 @@ } $dest_file = $save_dir . DIRECTORY_SEPARATOR . $save_as; + if (is_link($dest_file)) { + return PEAR::raiseError('SECURITY ERROR: Will not write to ' . $dest_file . ' as it is symlinked to ' . readlink($dest_file) . ' - Possible symlink attack'); + } + if (!$wp = @fopen($dest_file, 'wb')) { fclose($fp); if ($callback) { @@ -1758,5 +1763,4 @@ } return $dest_file; } -} -// }}} \ No newline at end of file +} \ No newline at end of file Index: PEAR/Downloader/Package.php =================================================================== --- PEAR/Downloader/Package.php (revision 308686) +++ PEAR/Downloader/Package.php (revision 308687) @@ -1376,6 +1376,8 @@ continue; } + // FIXME do symlink check + fwrite($fp, $filecontents, strlen($filecontents)); fclose($fp); if ($s = $params[$i]->explicitState()) { @@ -1497,13 +1499,12 @@ * @param int * @param string */ - function &getPackagefileObject(&$c, $d, $t = false) + function &getPackagefileObject(&$c, $d) { - $a = &new PEAR_PackageFile($c, $d, $t); + $a = &new PEAR_PackageFile($c, $d); return $a; } - /** * This will retrieve from a local file if possible, and parse out * a group name as well. The original parameter will be modified to reflect this. @@ -1527,16 +1528,7 @@ if (@is_file($param)) { $this->_type = 'local'; $options = $this->_downloader->getOptions(); - if (isset($options['downloadonly'])) { - $pkg = &$this->getPackagefileObject($this->_config, - $this->_downloader->_debug); - } else { - if (PEAR::isError($dir = $this->_downloader->getDownloadDir())) { - return $dir; - } - $pkg = &$this->getPackagefileObject($this->_config, - $this->_downloader->_debug, $dir); - } + $pkg = &$this->getPackagefileObject($this->_config, $this->_downloader->_debug); PEAR::pushErrorHandling(PEAR_ERROR_RETURN); $pf = &$pkg->fromAnyFile($param, PEAR_VALIDATE_INSTALLING); PEAR::popErrorHandling(); @@ -1608,15 +1600,7 @@ } // whew, download worked! - if (isset($options['downloadonly'])) { - $pkg = &$this->getPackagefileObject($this->_config, $this->_downloader->debug); - } else { - $dir = $this->_downloader->getDownloadDir(); - if (PEAR::isError($dir)) { - return $dir; - } - $pkg = &$this->getPackagefileObject($this->_config, $this->_downloader->debug, $dir); - } + $pkg = &$this->getPackagefileObject($this->_config, $this->_downloader->debug); PEAR::pushErrorHandling(PEAR_ERROR_RETURN); $pf = &$pkg->fromAnyFile($file, PEAR_VALIDATE_INSTALLING); Index: PEAR/REST.php =================================================================== --- PEAR/REST.php (revision 308686) +++ PEAR/REST.php (revision 308687) @@ -100,7 +100,10 @@ $ret = $this->getCache($url); if (!PEAR::isError($ret) && $trieddownload) { // reset the age of the cache if the server says it was unmodified - $this->saveCache($url, $ret, null, true, $cacheId); + $result = $this->saveCache($url, $ret, null, true, $cacheId); + if (PEAR::isError($result)) { + return PEAR::raiseErro($result->getMessage()); + } } return $ret; @@ -117,7 +120,11 @@ } if ($forcestring) { - $this->saveCache($url, $content, $lastmodified, false, $cacheId); + $result = $this->saveCache($url, $content, $lastmodified, false, $cacheId); + if (PEAR::isError($result)) { + return PEAR::raiseErro($result->getMessage()); + } + return $content; } @@ -153,7 +160,11 @@ $content = $parser->getData(); } - $this->saveCache($url, $content, $lastmodified, false, $cacheId); + $result = $this->saveCache($url, $content, $lastmodified, false, $cacheId); + if (PEAR::isError($result)) { + return PEAR::raiseErro($result->getMessage()); + } + return $content; } @@ -212,57 +223,65 @@ */ function saveCache($url, $contents, $lastmodified, $nochange = false, $cacheid = null) { - $cachedir = $this->config->get('cache_dir') . DIRECTORY_SEPARATOR . md5($url); - $cacheidfile = $cachedir . 'rest.cacheid'; - $cachefile = $cachedir . 'rest.cachefile'; + $cache_dir = $this->config->get('cache_dir'); + $d = $cache_dir . DIRECTORY_SEPARATOR . md5($url); + $cacheidfile = $d . 'rest.cacheid'; + $cachefile = $d . 'rest.cachefile'; if ($cacheid === null && $nochange) { $cacheid = unserialize(implode('', file($cacheidfile))); } - $fp = @fopen($cacheidfile, 'wb'); - if (!$fp) { - $cache_dir = $this->config->get('cache_dir'); + if (is_link($cacheidfile)) { + return PEAR::raiseError('SECURITY ERROR: Will not write to ' . $cacheidfile . ' as it is symlinked to ' . readlink($cacheidfile) . ' - Possible symlink attack'); + } + + if (is_link($cachefile)) { + return PEAR::raiseError('SECURITY ERROR: Will not write to ' . $cacheidfile . ' as it is symlinked to ' . readlink($cacheidfile) . ' - Possible symlink attack'); + } + + $cacheidfile_fp = @fopen($cacheidfile, 'wb'); + if (!$cacheidfile_fp) { if (is_dir($cache_dir)) { - return false; + return PEAR::raiseError("The value of config option cache_dir ($cache_dir) is not a directory. "); } System::mkdir(array('-p', $cache_dir)); - $fp = @fopen($cacheidfile, 'wb'); - if (!$fp) { - return false; + $cacheidfile_fp = @fopen($cacheidfile, 'wb'); + if (!$cacheidfile_fp) { + return PEAR::raiseError("Could not open $cacheidfile for writing."); } } if ($nochange) { - fwrite($fp, serialize(array( + fwrite($cacheidfile_fp, serialize(array( 'age' => time(), 'lastChange' => $cacheid['lastChange'], )) ); - fclose($fp); + fclose($cacheidfile_fp); return true; } - fwrite($fp, serialize(array( + fwrite($cacheidfile_fp, serialize(array( 'age' => time(), 'lastChange' => $lastmodified, )) ); + fclose($cacheidfile_fp); - fclose($fp); - $fp = @fopen($cachefile, 'wb'); - if (!$fp) { + $cachefile_fp = @fopen($cachefile, 'wb'); + if (!$cachefile_fp) { if (file_exists($cacheidfile)) { @unlink($cacheidfile); } - return false; + return PEAR::raiseError("Could not open $cacheidfile for writing."); } - fwrite($fp, serialize($contents)); - fclose($fp); + fwrite($cachefile_fp, serialize($contents)); + fclose($cachefile_fp); return true; } Index: PEAR/Builder.php =================================================================== --- PEAR/Builder.php (revision 308686) +++ PEAR/Builder.php (revision 308687) @@ -220,7 +220,7 @@ /** * Build an extension from source. Runs "phpize" in the source * directory, but compiles in a temporary directory - * (/var/tmp/pear-build-USER/PACKAGE-VERSION). + * (TMPDIR/pear-build-USER/PACKAGE-VERSION). * * @param string|PEAR_PackageFile_v* $descfile path to XML package description file, or * a PEAR_PackageFile object @@ -250,6 +250,7 @@ ' appears to have a prefix ' . $matches[2] . ', but' . ' config variable php_prefix does not match'); } + if (isset($matches[3]) && strlen($matches[3]) && trim($matches[3]) != trim($this->config->get('php_suffix'))) { $this->log(0, 'WARNING: php_bin ' . $this->config->get('php_bin') . @@ -258,14 +259,15 @@ } } - $this->current_callback = $callback; if (PEAR_OS == "Windows") { return $this->_build_win32($descfile, $callback); } + if (PEAR_OS != 'Unix') { return $this->raiseError("building extensions not supported on this platform"); } + if (is_object($descfile)) { $pkg = $descfile; $descfile = $pkg->getPackageFile(); @@ -284,14 +286,17 @@ } $dir = dirname($descfile); } + $old_cwd = getcwd(); if (!file_exists($dir) || !is_dir($dir) || !chdir($dir)) { return $this->raiseError("could not chdir to $dir"); } + $vdir = $pkg->getPackage() . '-' . $pkg->getVersion(); if (is_dir($vdir)) { chdir($vdir); } + $dir = getcwd(); $this->log(2, "building in $dir"); putenv('PATH=' . $this->config->get('bin_dir') . ':' . getenv('PATH')); @@ -302,6 +307,7 @@ if (PEAR::isError($err)) { return $err; } + if (!$err) { return $this->raiseError("`phpize' failed"); } @@ -327,30 +333,31 @@ // }}} end of interactive part // FIXME make configurable - if(!$user=getenv('USER')){ + if (!$user=getenv('USER')) { $user='defaultuser'; } - $build_basedir = "/var/tmp/pear-build-$user"; + + $tmpdir = $this->config->get('temp_dir'); + $build_basedir = System::mktemp(" -t $tmpdir -d pear-build-$user"); $build_dir = "$build_basedir/$vdir"; $inst_dir = "$build_basedir/install-$vdir"; $this->log(1, "building in $build_dir"); if (is_dir($build_dir)) { System::rm(array('-rf', $build_dir)); } + if (!System::mkDir(array('-p', $build_dir))) { return $this->raiseError("could not create build dir: $build_dir"); } + $this->addTempFile($build_dir); if (!System::mkDir(array('-p', $inst_dir))) { return $this->raiseError("could not create temporary install dir: $inst_dir"); } $this->addTempFile($inst_dir); - if (getenv('MAKE')) { - $make_command = getenv('MAKE'); - } else { - $make_command = 'make'; - } + $make_command = getenv('MAKE') ? getenv('MAKE') : 'make'; + $to_run = array( $configure_command, $make_command, Index: PEAR/Command/Remote.php =================================================================== --- PEAR/Command/Remote.php (revision 308686) +++ PEAR/Command/Remote.php (revision 308687) @@ -144,7 +144,7 @@ 'shortcut' => 'cc', 'options' => array(), 'doc' => ' -Clear the XML-RPC/REST cache. See also the cache_ttl configuration +Clear the REST cache. See also the cache_ttl configuration parameter. ', ), @@ -776,6 +776,7 @@ if ($verbose >= 1) { $output .= "reading directory $cache_dir\n"; } + $num = 0; while ($ent = readdir($dp)) { if (preg_match('/rest.cache(file|id)\\z/', $ent)) { Index: PEAR/Command/Package.php =================================================================== --- PEAR/Command/Package.php (revision 308686) +++ PEAR/Command/Package.php (revision 308687) @@ -314,7 +314,7 @@ return $a; } - function &getPackageFile($config, $debug = false, $tmpdir = null) + function &getPackageFile($config, $debug = false) { if (!class_exists('PEAR_Common')) { require_once 'PEAR/Common.php'; @@ -322,7 +322,7 @@ if (!class_exists('PEAR_PackageFile')) { require_once 'PEAR/PackageFile.php'; } - $a = &new PEAR_PackageFile($config, $debug, $tmpdir); + $a = &new PEAR_PackageFile($config, $debug); $common = new PEAR_Common; $common->ui = $this->ui; $a->setLogger($common); @@ -969,7 +969,9 @@ } $tar = new Archive_Tar($params[0]); - $tmpdir = System::mktemp('-d pearsign'); + + $tmpdir = $this->config->get('temp_dir'); + $tmpdir = System::mktemp(" -t $tmpdir -d pearsign"); if (!$tar->extractList('package2.xml package.xml package.sig', $tmpdir)) { return $this->raiseError("failed to extract tar file"); } Index: PEAR/Command/Install.php =================================================================== --- PEAR/Command/Install.php (revision 308686) +++ PEAR/Command/Install.php (revision 308687) @@ -730,7 +730,8 @@ if ($param->getPackageType() == 'extsrc' || $param->getPackageType() == 'extbin' || $param->getPackageType() == 'zendextsrc' || - $param->getPackageType() == 'zendextbin') { + $param->getPackageType() == 'zendextbin' + ) { $pkg = &$param->getPackageFile(); if ($instbin = $pkg->getInstalledBinary()) { $instpkg = &$instreg->getPackage($instbin, $pkg->getChannel()); @@ -741,7 +742,8 @@ foreach ($instpkg->getFilelist() as $name => $atts) { $pinfo = pathinfo($atts['installed_as']); if (!isset($pinfo['extension']) || - in_array($pinfo['extension'], array('c', 'h'))) { + in_array($pinfo['extension'], array('c', 'h')) + ) { continue; // make sure we don't match php_blah.h } Index: PEAR/Command/Pickle.php =================================================================== --- PEAR/Command/Pickle.php (revision 308686) +++ PEAR/Command/Pickle.php (revision 308687) @@ -104,7 +104,7 @@ * @param string|null $tmpdir * @return PEAR_PackageFile */ - function &getPackageFile($config, $debug = false, $tmpdir = null) + function &getPackageFile($config, $debug = false) { if (!class_exists('PEAR_Common')) { require_once 'PEAR/Common.php'; @@ -114,7 +114,7 @@ require_once 'PEAR/PackageFile.php'; } - $a = &new PEAR_PackageFile($config, $debug, $tmpdir); + $a = &new PEAR_PackageFile($config, $debug); $common = new PEAR_Common; $common->ui = $this->ui; $a->setLogger($common);