Skip to content

Commit

Permalink
[impr-OpenMage#442] Support symlinks while not allowing malicious tem…
Browse files Browse the repository at this point in the history
…plate paths

* Support symlinks while not allowing malicious template paths.
* Add protection for unauthorized file access to all fallback-based paths and
  allow symlinks for inlinecss directive.
* Remove symlink notification.
  • Loading branch information
colinmollenhour authored and edannenberg committed Aug 24, 2020
1 parent 02a1098 commit 88e9659
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 92 deletions.
36 changes: 0 additions & 36 deletions app/code/core/Mage/Adminhtml/Block/Notification/Symlink.php

This file was deleted.

27 changes: 10 additions & 17 deletions app/code/core/Mage/Core/Block/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ class Mage_Core_Block_Template extends Mage_Core_Block_Abstract

protected $_jsUrl;

/**
* Is allowed symlinks flag
*
* @var bool
*/
protected $_allowSymlinks = null;

protected static $_showTemplateHints;
protected static $_showTemplateHintsBlocks;

Expand Down Expand Up @@ -167,8 +160,7 @@ public function assign($key, $value=null)
*/
public function setScriptPath($dir)
{
$scriptPath = realpath($dir);
if (strpos($scriptPath, realpath(Mage::getBaseDir('design'))) === 0 || $this->_getAllowSymlinks()) {
if (strpos($dir, '..') === FALSE && ($dir === Mage::getBaseDir('design') || strpos(realpath($dir), realpath(Mage::getBaseDir('design'))) === 0)) {
$this->_viewDir = $dir;
} else {
Mage::log('Not valid script path:' . $dir, Zend_Log::CRIT, null, null, true);
Expand Down Expand Up @@ -236,9 +228,12 @@ public function fetchView($fileName)
}

try {
$includeFilePath = realpath($this->_viewDir . DS . $fileName);
if ($includeFilePath != '' && (strpos($includeFilePath, realpath($this->_viewDir)) === 0 || $this->_getAllowSymlinks())) {
include $includeFilePath;
if (
strpos($this->_viewDir . DS . $fileName, '..') === FALSE
&&
($this->_viewDir == Mage::getBaseDir('design') || strpos(realpath($this->_viewDir), realpath(Mage::getBaseDir('design'))) === 0)
) {
include $this->_viewDir . DS . $fileName;
} else {
$thisClass = get_class($this);
Mage::log('Not valid template file:' . $fileName . ' class: ' . $thisClass, Zend_Log::CRIT, null, true);
Expand Down Expand Up @@ -345,15 +340,13 @@ public function getCacheKeyInfo()
}

/**
* Get is allowed symliks flag
* Get is allowed symlinks flag
*
* @deprecated
* @return bool
*/
protected function _getAllowSymlinks()
{
if (is_null($this->_allowSymlinks)) {
$this->_allowSymlinks = Mage::getStoreConfigFlag(self::XML_PATH_TEMPLATE_ALLOW_SYMLINK);
}
return $this->_allowSymlinks;
return FALSE;
}
}
16 changes: 16 additions & 0 deletions app/code/core/Mage/Core/Model/Design/Package.php
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,18 @@ protected function _fallback($file, array &$params, array $fallbackScheme = arra
* @param string $file
* @param array $params
* @return string
* @throws Exception
*/
public function getFilename($file, array $params)
{
Varien_Profiler::start(__METHOD__);

// Prevent reading files outside of the proper directory while still allowing symlinked files
if (strpos($file, '..') !== false) {
Mage::log(sprintf('Invalid path requested: %s (params: %s)', $file, json_encode($params)), Zend_Log::ERR);
throw new Exception('Invalid path requested.');
}

$this->updateParamDefaults($params);
$result = $this->_fallback(
$file,
Expand Down Expand Up @@ -478,10 +486,18 @@ public function getLocaleFileName($file, array $params=array())
* @param string $file
* @param array $params
* @return string
* @throws Exception
*/
public function getSkinUrl($file = null, array $params = array())
{
Varien_Profiler::start(__METHOD__);

// Prevent reading files outside of the proper directory while still allowing symlinked files
if (strpos($file, '..') !== false) {
Mage::log(sprintf('Invalid path requested: %s (params: %s)', $file, json_encode($params)), Zend_Log::ERR);
throw new Exception('Invalid path requested.');
}

if (empty($params['_type'])) {
$params['_type'] = 'skin';
}
Expand Down
4 changes: 1 addition & 3 deletions app/code/core/Mage/Core/Model/Email/Template/Abstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,9 @@ protected function _getCssFileContent($filename)
'_theme' => $theme,
)
);
$filePath = realpath($filePath);
$positionSkinDirectory = strpos($filePath, Mage::getBaseDir('skin'));
$validator = new Zend_Validate_File_Extension('css');

if ($validator->isValid($filePath) && $positionSkinDirectory !== false && is_readable($filePath)) {
if ($validator->isValid($filePath) && is_readable($filePath)) {
return (string) file_get_contents($filePath);
}

Expand Down
2 changes: 1 addition & 1 deletion app/code/core/Mage/Page/Block/Html/Topmenu/Renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected function _toHtml()
}

$includeFilePath = realpath(Mage::getBaseDir('design') . DS . $this->getTemplateFile());
if (strpos($includeFilePath, realpath(Mage::getBaseDir('design'))) === 0 || $this->_getAllowSymlinks()) {
if (strpos($this->getTemplateFile(), '..') === FALSE) {
$this->_templateFile = $includeFilePath;
} else {
throw new Exception('Not valid template file:' . $this->_templateFile);
Expand Down
1 change: 0 additions & 1 deletion app/design/adminhtml/default/default/layout/main.xml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ Default layout, loads most of the pages
<block type="adminhtml/notification_survey" name="notification_survey" template="notification/survey.phtml"/>
<block type="adminhtml/notification_security" name="notification_security" as="notification_security" template="notification/security.phtml"></block>
<block type="adminhtml/checkout_formkey" name="checkout_formkey" as="checkout_formkey" template="notification/formkey.phtml"/>
<block type="adminhtml/notification_symlink" name="notification_symlink" template="notification/symlink.phtml"/>
</block>
<block type="adminhtml/widget_breadcrumbs" name="breadcrumbs" as="breadcrumbs"></block>

Expand Down

This file was deleted.

0 comments on commit 88e9659

Please sign in to comment.