From bc30a5fb67ad0ef8cff0f38fc6ddd65a7d3beff4 Mon Sep 17 00:00:00 2001 From: Tymoteusz Motylewski Date: Wed, 29 Oct 2014 10:46:55 +0100 Subject: [PATCH] SUPEE-3941 - Addresses extension-related issues This is an official Magento patch. * When you install a community-created translation package, the translation provided by the package overwrites any existing translations for the same items. This enables you to more easily install packages with translations. * To improve security, Magento Connect now uses HTTPS by default to download extensions, rather than FTP. * Extension developers can now create an extensions with a dash character in the name. Merchants can install those extensions without issues. * Magento administrators who attempt to install an extension with insufficient file system privileges are now informed. Typically, the Magento Admin Panel runs as the web server user. If this user has insufficient privileges to the your Magento install dir/app/code/community directory structure, the Magento administrator sees an error message in the Magento Connect Manager. To set file system permissions appropriately, see http://www.magentocommerce.com/knowledge-base/entry/install-privs-after#extensions --- downloader/Maged/Model/Connect.php | 5 +- downloader/lib/Mage/Connect/Backup.php | 169 ++++++++++++++++++ downloader/lib/Mage/Connect/Command.php | 24 ++- .../lib/Mage/Connect/Command/Install.php | 38 +++- downloader/lib/Mage/Connect/Packager.php | 90 ++++++++-- downloader/lib/Mage/Connect/Rest.php | 7 +- downloader/lib/Mage/Connect/Singleconfig.php | 1 - downloader/lib/Mage/Connect/Validator.php | 6 +- downloader/lib/Mage/HTTP/Client/Curl.php | 100 ++++++----- downloader/template/settings.phtml | 2 +- 10 files changed, 369 insertions(+), 73 deletions(-) create mode 100644 downloader/lib/Mage/Connect/Backup.php diff --git a/downloader/Maged/Model/Connect.php b/downloader/Maged/Model/Connect.php index 4039910cd7f..d1bb3f7e0c6 100644 --- a/downloader/Maged/Model/Connect.php +++ b/downloader/Maged/Model/Connect.php @@ -486,6 +486,9 @@ public function saveConfigPost($p) */ public function checkExtensionKey($id, &$match) { - return preg_match('#^([^ ]+)\/([^-]+)(-.+)?$#', $id, $match); + if (preg_match('#^(.+)\/(.+)-([\.\d]+)$#', $id, $match)) { + return $match; + } + return preg_match('#^(.+)\/(.+)$#', $id, $match); } } diff --git a/downloader/lib/Mage/Connect/Backup.php b/downloader/lib/Mage/Connect/Backup.php new file mode 100644 index 00000000000..862022562fc --- /dev/null +++ b/downloader/lib/Mage/Connect/Backup.php @@ -0,0 +1,169 @@ + + */ +class Mage_Connect_Backup +{ + /** + * Prefix for backuped files + * + * @var string + */ + protected $_prefix = '_backup_'; + + /** + * Array of available to overwrite type of files + * + * @var array + */ + protected $_fileTypes = array(); + + /** + * List of files to backup files + * + * @var array + */ + protected $_fileList = array(); + + /** + * Get available file types for backup + * + * @return array + */ + public function getFileTypes() + { + return $this->_fileTypes; + } + + /** + * Set available file types for backup + * + * @param array $types + */ + public function setFileTypes(array $types) + { + foreach ($types as $type) { + $this->_fileTypes[] = $type; + } + } + + /** + * Add file to files list for backup + * + * @param string $file + * @param string $rootPath + * @return void + */ + public function addFile($file, $rootPath) + { + $dest = $rootPath . DS . $file; + $type = $this->getFileType($file); + if (file_exists($dest) && in_array($type, $this->getFileTypes())) { + $this->_fileList[] = $file; + } + } + + /** + * Get count of files + * + * @return int + */ + public function getFilesCount() + { + return count($this->_fileList); + } + + /** + * Clear list of files + * + * @return void + */ + public function unsetAllFiles() + { + $this->_fileList = array(); + } + + /** + * Get list of files + * + * @return array + */ + public function getAllFiles() + { + return $this->_fileList; + } + + /** + * Run backup process + * + * @param boolean $cleanUpQueue + * @return void + */ + public function run($cleanUpQueue = false) + { + if ($this->getFilesCount() > 0) { + $fileList = $this->getAllFiles(); + foreach($fileList as $file) { + $this->_backupFile($file); + } + if ($cleanUpQueue) { + $this->unsetAllFiles(); + } + } + } + + /** + * Get File type + * + * @param string $file + * @return string + */ + public function getFileType($file) + { + return pathinfo($file, PATHINFO_EXTENSION); + } + + /** + * Backup file + * + * @param string $file + * @return void + */ + private function _backupFile($file) + { + $type = $this->getFileType($file); + if ($type && $type != '') { + $newName = $this->_prefix . time() . '.' . $type; + @rename($file, str_replace('.' . $type, $newName, $file)); + } + } +} diff --git a/downloader/lib/Mage/Connect/Command.php b/downloader/lib/Mage/Connect/Command.php index 588f48e1d4d..4a1d36a4c9a 100644 --- a/downloader/lib/Mage/Connect/Command.php +++ b/downloader/lib/Mage/Connect/Command.php @@ -63,6 +63,13 @@ class Mage_Connect_Command */ protected static $_validator = null; + /** + * Backup instance + * + * @var Mage_Connect_Backup + */ + protected static $_backup = null; + /** * Rest instance * @@ -248,6 +255,19 @@ public function validator() return self::$_validator; } + /** + * Get backup object + * + * @return Mage_Connect_Backup + */ + public function backup() + { + if(is_null(self::$_backup)) { + self::$_backup = new Mage_Connect_Backup(); + } + return self::$_backup; + } + /** * Get rest object * @@ -422,8 +442,8 @@ public function splitPackageArgs(array & $params) return; } if(preg_match("@([a-zA-Z0-9_]+)/([a-zA-Z0-9_]+)@ims", $params[0], $subs)) { - $params[0] = $subs[2]; - array_unshift($params, $subs[1]); + $params[0] = $subs[2]; + array_unshift($params, $subs[1]); } } diff --git a/downloader/lib/Mage/Connect/Command/Install.php b/downloader/lib/Mage/Connect/Command/Install.php index 8cd8e3f62bf..81897fd5ef6 100644 --- a/downloader/lib/Mage/Connect/Command/Install.php +++ b/downloader/lib/Mage/Connect/Command/Install.php @@ -90,15 +90,15 @@ public function doInstall($command, $options, $params, $objects = array()) @mkdir($config->magento_root . $dirTmp,0777,true); @mkdir($config->magento_root . $dirMedia,0777,true); $isWritable = is_writable($config->magento_root) - && is_writable($config->magento_root . DIRECTORY_SEPARATOR . $config->downloader_path) - && is_writable($config->magento_root . $dirCache) - && is_writable($config->magento_root . $dirTmp) - && is_writable($config->magento_root . $dirMedia); + && is_writable($config->magento_root . DIRECTORY_SEPARATOR . $config->downloader_path) + && is_writable($config->magento_root . $dirCache) + && is_writable($config->magento_root . $dirTmp) + && is_writable($config->magento_root . $dirMedia); $err = "Please check for sufficient write file permissions."; } $isWritable = $isWritable && is_writable($config->magento_root . $dirMedia) - && is_writable($config->magento_root . $dirCache) - && is_writable($config->magento_root . $dirTmp); + && is_writable($config->magento_root . $dirCache) + && is_writable($config->magento_root . $dirTmp); if (!$isWritable) { $this->doError($command, $err); throw new Exception( @@ -316,7 +316,7 @@ public function doInstall($command, $options, $params, $objects = array()) if ($ftp) { $cwd=$ftpObj->getcwd(); $dir=$cwd . DIRECTORY_SEPARATOR .$config->downloader_path . DIRECTORY_SEPARATOR - . Mage_Connect_Config::DEFAULT_CACHE_PATH . DIRECTORY_SEPARATOR . trim( $pChan, "\\/"); + . Mage_Connect_Config::DEFAULT_CACHE_PATH . DIRECTORY_SEPARATOR . trim( $pChan, "\\/"); $ftpObj->mkdirRecursive($dir,0777); $ftpObj->chdir($cwd); } else { @@ -346,11 +346,33 @@ public function doInstall($command, $options, $params, $objects = array()) $package = new Mage_Connect_Package($file); if ($clearInstallMode && $pInstallState != 'upgrade' && !$installAll) { - $this->validator()->validateContents($package->getContents(), $config); + $contents = $package->getContents(); + $this->backup()->setFileTypes(array('csv', 'html')); + $typesToBackup = $this->backup()->getFileTypes(); + $this->validator()->validateContents($contents, $config, $typesToBackup); $errors = $this->validator()->getErrors(); if (count($errors)) { throw new Exception("Package '{$pName}' is invalid\n" . implode("\n", $errors)); } + + $targetPath = rtrim($config->magento_root, "\\/"); + foreach ($contents as $filePath) { + $this->backup()->addFile($filePath, $targetPath); + } + + if ($this->backup()->getFilesCount() > 0) { + $this->ui()->output('
'); + $this->ui()->output('Backup of following files will be created :'); + $this->ui()->output('
'); + $this->backup()->run(); + $this->ui()->output(implode('
', $this->backup()->getAllFiles())); + $this->ui()->output('
'); + $this->ui()->output( + $this->backup()->getFilesCount() . ' files was overwritten by installed extension.' + ); + $this->ui()->output('
'); + $this->backup()->unsetAllFiles(); + } } $conflicts = $package->checkPhpDependencies(); diff --git a/downloader/lib/Mage/Connect/Packager.php b/downloader/lib/Mage/Connect/Packager.php index 3336a8aa62f..f269bc03de5 100644 --- a/downloader/lib/Mage/Connect/Packager.php +++ b/downloader/lib/Mage/Connect/Packager.php @@ -215,7 +215,7 @@ public function writeToRemoteCache($cache, $ftpObj) * * @param Mage_Connect_Config $cache * @param Mage_Connect_Ftp $ftpObj - * @return void + * @throws RuntimeException */ public function writeToRemoteConfig($cache, $ftpObj) { @@ -240,8 +240,15 @@ protected function removeEmptyDirectory($dir, $ftp = null) } } } else { - if (@rmdir($dir)) { - $this->removeEmptyDirectory(dirname($dir), $ftp); + $content = scandir($dir); + if ($content === false) return; + + if (count(array_diff($content, array('.', '..'))) == 0) { + if (@rmdir($dir)) { + $this->removeEmptyDirectory(dirname($dir), $ftp); + } else { + throw new RuntimeException('Failed to delete dir ' . $dir . "\r\n Check permissions"); + } } } } @@ -253,7 +260,7 @@ protected function removeEmptyDirectory($dir, $ftp = null) * @param string $package * @param Mage_Connect_Singleconfig $cacheObj * @param Mage_Connect_Config $configObj - * @return void + * @throws RuntimeException */ public function processUninstallPackage($chanName, $package, $cacheObj, $configObj) { @@ -261,14 +268,21 @@ public function processUninstallPackage($chanName, $package, $cacheObj, $configO $contents = $package->getContents(); $targetPath = rtrim($configObj->magento_root, "\\/"); + $failedFiles = array(); foreach ($contents as $file) { $fileName = basename($file); $filePath = dirname($file); $dest = $targetPath . DIRECTORY_SEPARATOR . $filePath . DIRECTORY_SEPARATOR . $fileName; if(@file_exists($dest)) { - @unlink($dest); - $this->removeEmptyDirectory(dirname($dest)); + if (!@unlink($dest)) { + $failedFiles[] = $dest; + } } + $this->removeEmptyDirectory(dirname($dest)); + } + if (!empty($failedFiles)) { + $msg = sprintf("Failed to delete files: %s \r\n Check permissions", implode("\r\n", $failedFiles)); + throw new RuntimeException($msg); } $destDir = $targetPath . DS . Mage_Connect_Package::PACKAGE_XML_DIR; @@ -283,17 +297,26 @@ public function processUninstallPackage($chanName, $package, $cacheObj, $configO * @param $package * @param Mage_Connect_Singleconfig $cacheObj * @param Mage_Connect_Ftp $ftp - * @return void + * @throws RuntimeException */ public function processUninstallPackageFtp($chanName, $package, $cacheObj, $ftp) { $ftpDir = $ftp->getcwd(); $package = $cacheObj->getPackageObject($chanName, $package); $contents = $package->getContents(); + $failedFiles = array(); foreach ($contents as $file) { $ftp->delete($file); + if ($ftp->fileExists($file)) { + $failedFiles[] = $file; + continue; + } $this->removeEmptyDirectory(dirname($file), $ftp); } + if (!empty($failedFiles)) { + $msg = sprintf("Failed to delete files: %s \r\n Check permissions", implode("\r\n", $failedFiles)); + throw new RuntimeException($msg); + } $remoteXml = Mage_Connect_Package::PACKAGE_XML_DIR . DS . $package->getReleaseFilename() . '.xml'; $ftp->delete($remoteXml); $ftp->chdir($ftpDir); @@ -362,7 +385,7 @@ protected function convertFtpPath($str) * @param string $file * @param Mage_Connect_Config $configObj * @param Mage_Connect_Ftp $ftp - * @return void + * @throws RuntimeException */ public function processInstallPackageFtp($package, $file, $configObj, $ftp) { @@ -370,10 +393,13 @@ public function processInstallPackageFtp($package, $file, $configObj, $ftp) $contents = $package->getContents(); $arc = $this->getArchiver(); $target = dirname($file) . DS . $package->getReleaseFilename(); - @mkdir($target, 0777, true); + if (!@mkdir($target, 0777, true)) { + throw new RuntimeException("Can't create directory ". $target); + } $tar = $arc->unpack($file, $target); $modeFile = $this->_getFileMode($configObj); $modeDir = $this->_getDirMode($configObj); + $failedFiles = array(); foreach ($contents as $file) { $source = $tar . DS . $file; if (file_exists($source) && is_file($source)) { @@ -382,10 +408,17 @@ public function processInstallPackageFtp($package, $file, $configObj, $ftp) $args[] = $modeDir; $args[] = $modeFile; } - call_user_func_array(array($ftp,'upload'), $args); + if (call_user_func_array(array($ftp,'upload'), $args) === false) { + $failedFiles[] = $source; + } } } + if (!empty($failedFiles)) { + $msg = sprintf("Failed to upload files: %s \r\n Check permissions", implode("\r\n", $failedFiles)); + throw new RuntimeException($msg); + } + $localXml = $tar . Mage_Connect_Package_Reader::DEFAULT_NAME_PACKAGE; if (is_file($localXml)) { $remoteXml = Mage_Connect_Package::PACKAGE_XML_DIR . DS . $package->getReleaseFilename() . '.xml'; @@ -414,11 +447,16 @@ public function processInstallPackage($package, $file, $configObj) $modeFile = $this->_getFileMode($configObj); $modeDir = $this->_getDirMode($configObj); $targetPath = rtrim($configObj->magento_root, "\\/"); + $packageXmlDir = $targetPath . DS . Mage_Connect_Package::PACKAGE_XML_DIR; + if (!is_dir_writeable($packageXmlDir)) { + throw new RuntimeException('Directory ' . $packageXmlDir . ' is not writable. Check permission'); + } + $this->_makeDirectories($contents, $targetPath, $modeDir); foreach ($contents as $file) { $fileName = basename($file); $filePath = dirname($file); $source = $tar . DS . $file; - @mkdir($targetPath. DS . $filePath, $modeDir, true); + $source = $tar . DS . $file; $dest = $targetPath . DS . $filePath . DS . $fileName; if (is_file($source)) { @copy($source, $dest); @@ -443,6 +481,36 @@ public function processInstallPackage($package, $file, $configObj) Mage_System_Dirs::rm(array("-r",$target)); } + /** + * @param array $content + * @param string $targetPath + * @param int $modeDir + * @throws RuntimeException + */ + protected function _makeDirectories($content, $targetPath, $modeDir) + { + $failedDirs = array(); + $createdDirs = array(); + foreach ($content as $file) { + $dirPath = dirname($file); + if (is_dir($dirPath) && is_dir_writeable($dirPath)) { + continue; + } + if (!mkdir($targetPath . DS . $dirPath, $modeDir, true)) { + $failedDirs[] = $targetPath . DS . $dirPath; + } else { + $createdDirs[] = $targetPath . DS . $dirPath; + } + } + if (!empty($failedDirs)) { + foreach ($createdDirs as $createdDir) { + $this->removeEmptyDirectory($createdDir); + } + $msg = sprintf("Failed to create directory:\r\n%s\r\n Check permissions", implode("\r\n", $failedDirs)); + throw new RuntimeException($msg); + } + } + /** * Get local modified files * diff --git a/downloader/lib/Mage/Connect/Rest.php b/downloader/lib/Mage/Connect/Rest.php index 7c58ecbbf5f..9c553154e82 100644 --- a/downloader/lib/Mage/Connect/Rest.php +++ b/downloader/lib/Mage/Connect/Rest.php @@ -82,17 +82,14 @@ class Mage_Connect_Rest * * @param string $protocol */ - public function __construct($protocol="http") + public function __construct($protocol="https") { switch ($protocol) { - case 'ftp': - $this->_protocol = 'ftp'; - break; case 'http': $this->_protocol = 'http'; break; default: - $this->_protocol = 'http'; + $this->_protocol = 'https'; break; } } diff --git a/downloader/lib/Mage/Connect/Singleconfig.php b/downloader/lib/Mage/Connect/Singleconfig.php index ef1131ebac5..706af6e40b2 100644 --- a/downloader/lib/Mage/Connect/Singleconfig.php +++ b/downloader/lib/Mage/Connect/Singleconfig.php @@ -136,7 +136,6 @@ public function formatUri($uri) $uri = rtrim($uri, "/"); $uri = str_replace("http://", '', $uri); $uri = str_replace("https://", '', $uri); - $uri = str_replace("ftp://", '', $uri); return $uri; } diff --git a/downloader/lib/Mage/Connect/Validator.php b/downloader/lib/Mage/Connect/Validator.php index d28cb08d6e8..14523069490 100644 --- a/downloader/lib/Mage/Connect/Validator.php +++ b/downloader/lib/Mage/Connect/Validator.php @@ -459,9 +459,10 @@ public function validatePHPVersion($min, $max, $ver = PHP_VERSION) * * @param array $contents * @param Mage_Connect_Config $config + * @param array $typesToBackup * @return bool */ - public function validateContents(array $contents, $config) + public function validateContents(array $contents, $config, $typesToBackup = array()) { if (!count($contents)) { $this->addError('Empty package contents section'); @@ -471,7 +472,8 @@ public function validateContents(array $contents, $config) $targetPath = rtrim($config->magento_root, "\\/"); foreach ($contents as $file) { $dest = $targetPath . DS . $file; - if (file_exists($dest)) { + $type = pathinfo($file, PATHINFO_EXTENSION); + if (file_exists($dest) && !in_array($type, $typesToBackup)) { $this->addError("'{$file}' already exists"); return false; } diff --git a/downloader/lib/Mage/HTTP/Client/Curl.php b/downloader/lib/Mage/HTTP/Client/Curl.php index c55f88da9d0..17a60a991ae 100644 --- a/downloader/lib/Mage/HTTP/Client/Curl.php +++ b/downloader/lib/Mage/HTTP/Client/Curl.php @@ -361,47 +361,20 @@ public function getStatus() /** * Make request + * * @param string $method * @param string $uri * @param array $params - * @return null + * @param boolean $isAuthorizationRequired */ - protected function makeRequest($method, $uri, $params = array()) + protected function makeRequest($method, $uri, $params = array(), $isAuthorizationRequired = true) { - static $isAuthorizationRequired = 0; + $uriModified = $this->getSecureRequest($uri, $isAuthorizationRequired); $this->_ch = curl_init(); - - // make request via secured layer - if ($isAuthorizationRequired && strpos($uri, 'https://') !== 0) { - $uri = str_replace('http://', '', $uri); - $uri = 'https://' . $uri; - } - - $this->curlOption(CURLOPT_URL, $uri); - $this->curlOption(CURLOPT_SSL_VERIFYPEER, FALSE); + $this->curlOption(CURLOPT_URL, $uriModified); + $this->curlOption(CURLOPT_SSL_VERIFYPEER, false); $this->curlOption(CURLOPT_SSL_VERIFYHOST, 2); - - // force method to POST if secured - if ($isAuthorizationRequired) { - $method = 'POST'; - } - - if($method == 'POST') { - $this->curlOption(CURLOPT_POST, 1); - $postFields = is_array($params) ? $params : array(); - if ($isAuthorizationRequired) { - $this->curlOption(CURLOPT_COOKIEJAR, self::COOKIE_FILE); - $this->curlOption(CURLOPT_COOKIEFILE, self::COOKIE_FILE); - $postFields = array_merge($postFields, $this->_auth); - } - if (!empty($postFields)) { - $this->curlOption(CURLOPT_POSTFIELDS, $postFields); - } - } elseif($method == "GET") { - $this->curlOption(CURLOPT_HTTPGET, 1); - } else { - $this->curlOption(CURLOPT_CUSTOMREQUEST, $method); - } + $this->getCurlMethodSettings($method, $params, $isAuthorizationRequired); if(count($this->_headers)) { $heads = array(); @@ -444,23 +417,26 @@ protected function makeRequest($method, $uri, $params = array()) $this->doError(curl_error($this->_ch)); } if(!$this->getStatus()) { - return $this->doError("Invalid response headers returned from server."); + $this->doError("Invalid response headers returned from server."); + return; } + curl_close($this->_ch); + if (403 == $this->getStatus()) { - if (!$isAuthorizationRequired) { - $isAuthorizationRequired++; - $this->makeRequest($method, $uri, $params); - $isAuthorizationRequired=0; + if ($isAuthorizationRequired) { + $this->makeRequest($method, $uri, $params, false); } else { - return $this->doError(sprintf('Access denied for %s@%s', $_SESSION['auth']['login'], $uri)); + $this->doError(sprintf('Access denied for %s@%s', $_SESSION['auth']['login'], $uriModified)); + return; } + } elseif (405 == $this->getStatus()) { + $this->doError("HTTP Error 405 Method not allowed"); + return; } } /** - * Throw error excpetion - * @param $string * @throws Exception */ public function isAuthorizationRequired() @@ -553,4 +529,44 @@ public function setOption($name, $value) { $this->_curlUserOptions[$name] = $value; } + + /** + * @param $uri + * @param $isAuthorizationRequired + * @return string + */ + protected function getSecureRequest($uri, $isAuthorizationRequired = true) + { + if ($isAuthorizationRequired && strpos($uri, 'https://') !== 0) { + $uri = str_replace('http://', '', $uri); + $uri = 'https://' . $uri; + return $uri; + } + return $uri; + } + + /** + * @param $method + * @param $params + * @param $isAuthorizationRequired + */ + protected function getCurlMethodSettings($method, $params, $isAuthorizationRequired) + { + if ($method == 'POST') { + $this->curlOption(CURLOPT_POST, 1); + $postFields = is_array($params) ? $params : array(); + if ($isAuthorizationRequired) { + $this->curlOption(CURLOPT_COOKIEJAR, self::COOKIE_FILE); + $this->curlOption(CURLOPT_COOKIEFILE, self::COOKIE_FILE); + $postFields = array_merge($postFields, $this->_auth); + } + if (!empty($postFields)) { + $this->curlOption(CURLOPT_POSTFIELDS, $postFields); + } + } elseif ($method == "GET") { + $this->curlOption(CURLOPT_HTTPGET, 1); + } else { + $this->curlOption(CURLOPT_CUSTOMREQUEST, $method); + } + } } diff --git a/downloader/template/settings.phtml b/downloader/template/settings.phtml index b91b41e18cc..9c7185e8eab 100755 --- a/downloader/template/settings.phtml +++ b/downloader/template/settings.phtml @@ -63,8 +63,8 @@ function changeDeploymentType (element) Magento Connect Channel Protocol: