Skip to content

Commit

Permalink
Dev: security fix, use libxml_disable_entity_loader for admin theme a…
Browse files Browse the repository at this point in the history 
…nd survey templates configuration files
  • Loading branch information
LouisGac committed Jul 22, 2016
1 parent 5a39a5d commit fe57ea3
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 41 deletions.
6 changes: 3 additions & 3 deletions application/helpers/replacements_helper.php
View file
Expand Up @@ -197,7 +197,7 @@ function templatereplace($line, $replacements = array(), &$redata = array(), $de
{
$surveyformat = "";
}
if($oTemplate->config->engine->cssframework)
if( isset($oTemplate->config->engine->cssframework) && $oTemplate->config->engine->cssframework)

This comment has been minimized.

Sign in to view

Copy link
@maziminke

maziminke Jul 28, 2016

Collaborator

This causes a fatal error when using a custom template, see http://ls25.mysurveyhosting.com/index.php/survey/index/sid/936162/newtest/Y/lang/de

This comment has been minimized.

Sign in to view

Copy link
@olleharstedt

olleharstedt Jul 28, 2016 •
edited

Collaborator

I guess it needs to be if isset($otemplate) && isset($otemplate->config) etc

This comment has been minimized.

Sign in to view

Copy link
@LouisGac

LouisGac Jul 28, 2016

Contributor

it can be removed (just avoid empty entries in config.xml).

This comment has been minimized.

Sign in to view

Copy link
@Shnoulle

Shnoulle Jul 28, 2016

Collaborator

@maziminke Usin a specific config or the minimal-config ?

I think : if we have $otemplate then we have $otemplate->config . no ?
{
$surveyformat .= " ".$oTemplate->config->engine->cssframework."-engine ";
}
Expand Down Expand Up @@ -556,12 +556,12 @@ function templatereplace($line, $replacements = array(), &$redata = array(), $de
if($thissurvey['googleanalyticsapikey'] === "9999useGlobal9999")
{
$_googleAnalyticsAPIKey = trim(getGlobalSetting('googleanalyticsapikey'));
}
}
else if (isset($thissurvey['googleanalyticsapikey']) && trim($thissurvey['googleanalyticsapikey']) != '')
{
$_googleAnalyticsAPIKey = trim($thissurvey['googleanalyticsapikey']);
}
else
else
{
$_googleAnalyticsAPIKey = "";

Expand Down
30 changes: 25 additions & 5 deletions application/models/AdminTheme.php
View file
Expand Up @@ -94,14 +94,25 @@ public function setAdminTheme()
// TODO: replace everywhere the call to Yii::app()->getConfig('adminstyleurl) by $oAdminTheme->sTemplateUrl;
Yii::app()->setConfig('adminstyleurl', $this->sTemplateUrl );

// We load the admin theme's configuration file.
$this->config = simplexml_load_file($this->path.'/config.xml');

//////////////////////
// Config file loading

$bOldEntityLoaderState = libxml_disable_entity_loader(true); // @see: http://phpsecurity.readthedocs.io/en/latest/Injection-Attacks.html#xml-external-entity-injection
$sXMLConfigFile = file_get_contents( realpath ($this->path.'/config.xml')); // Now that entity loader is disabled, we can't use simplexml_load_file; so we must read the file with file_get_contents and convert it as a string

// Simple Xml is buggy on PHP < 5.4. The [ array -> json_encode -> json_decode ] workaround seems to be the most used one.
// @see: http://php.net/manual/de/book.simplexml.php#105330 (top comment on PHP doc for simplexml)
$this->config = json_decode( json_encode ( ( array ) simplexml_load_string($sXMLConfigFile), 1));

// If developers want to test asset manager with debug mode on
$this->use_asset_manager = ( $this->config->engine->use_asset_manager_in_debug_mode == 'true');
$this->use_asset_manager = isset($this->config->engine->use_asset_manager_in_debug_mode)?( $this->config->engine->use_asset_manager_in_debug_mode == 'true'):'false';

$this->defineConstants(); // Define the (still) necessary constants
$this->registerStylesAndScripts(); // Register all CSS and JS

libxml_disable_entity_loader($bOldEntityLoaderState); // Put back entity loader to its original state, to avoid contagion to other applications on the server

return $this;
}

Expand Down Expand Up @@ -353,6 +364,8 @@ public static function getOtherAssets()
'application/extensions/SettingsWidget/assets',
'application/extensions/FlashMessage/assets',
'application/extensions/admin/survey/question/PositionWidget/assets',
'application/extensions/admin/grid/MassiveActionsWidget/assets',
'application/extensions/admin/survey/question/PositionWidget/assets',
//'application/extensions/bootstrap/', we'll touch all the subdirectories of extensions

// Third party assets
Expand All @@ -369,19 +382,26 @@ public static function getOtherAssets()
*/
static private function getThemeList($sDir)
{
$bOldEntityLoaderState = libxml_disable_entity_loader(true); // @see: http://phpsecurity.readthedocs.io/en/latest/Injection-Attacks.html#xml-external-entity-injection
$aListOfFiles = array();
if ($sDir && $pHandle = opendir($sDir))
{
while (false !== ($file = readdir($pHandle)))
{
if (is_dir($sDir.DIRECTORY_SEPARATOR.$file) && is_file($sDir.DIRECTORY_SEPARATOR.$file.DIRECTORY_SEPARATOR.'config.xml'))
{
$oTemplateConfig = simplexml_load_file($sDir.DIRECTORY_SEPARATOR.$file.'/config.xml');
$sXMLConfigFile = file_get_contents( realpath ($sDir.DIRECTORY_SEPARATOR.$file.'/config.xml')); // Now that entity loader is disabled, we can't use simplexml_load_file; so we must read the file with file_get_contents and convert it as a string

// Simple Xml is buggy on PHP < 5.4. The [ array -> json_encode -> json_decode ] workaround seems to be the most used one.
// @see: http://php.net/manual/de/book.simplexml.php#105330 (top comment on PHP doc for simplexml)
$oTemplateConfig = json_decode( json_encode ( ( array ) simplexml_load_string($sXMLConfigFile), 1));

$aListOfFiles[$file] = $oTemplateConfig;
}
}
closedir($pHandle);
}
libxml_disable_entity_loader($bOldEntityLoaderState);
return $aListOfFiles;
}

Expand All @@ -401,7 +421,7 @@ private function defineConstants()
}

// Define presentation text on welcome page
if($this->config->metadatas->presentation)
if (isset($this->config->metadatas->presentation) && $this->config->metadatas->presentation)
{
define('PRESENTATION', $this->config->metadatas->presentation);
}
Expand Down
61 changes: 28 additions & 33 deletions application/models/TemplateConfiguration.php
View file
Expand Up @@ -69,24 +69,15 @@ public function setTemplateConfiguration($sTemplateName='', $iSurveyId='')

if ($sTemplateName=='')
{
$this->oSurvey = Survey::model()->findByPk($iSurveyId);
$this->oSurvey = Survey::model()->findByPk($iSurveyId);
$this->sTemplateName = $this->oSurvey->template;
}

// We check if it's a CORE template
$this->isStandard = $this->setIsStandard();

// If the template is standard, its root is based on standardtemplaterootdir
if($this->isStandard)
{
$this->path = Yii::app()->getConfig("standardtemplaterootdir").DIRECTORY_SEPARATOR.$this->sTemplateName;
}
// Else, it's a user template, its root is based on usertemplaterootdir
else
{
$this->path = Yii::app()->getConfig("usertemplaterootdir").DIRECTORY_SEPARATOR.$this->sTemplateName;
}

// If the template is standard, its root is based on standardtemplaterootdir, else, it's a user template, its root is based on usertemplaterootdir
$this->path = ($this->isStandard)?Yii::app()->getConfig("standardtemplaterootdir").DIRECTORY_SEPARATOR.$this->sTemplateName:Yii::app()->getConfig("usertemplaterootdir").DIRECTORY_SEPARATOR.$this->sTemplateName;

// If the template directory doesn't exist, it can be that:
// - user deleted a custom theme
Expand All @@ -99,7 +90,6 @@ public function setTemplateConfiguration($sTemplateName='', $iSurveyId='')
setGlobalSetting('defaulttemplate', 'default');
}


// If the template don't have a config file (maybe it has been deleted, or whatever),
// then, we load the default template
$this->hasConfigFile = (string) is_file($this->path.DIRECTORY_SEPARATOR.'config.xml');
Expand All @@ -117,36 +107,41 @@ public function setTemplateConfiguration($sTemplateName='', $iSurveyId='')
$this->path = Yii::app()->getConfig("standardtemplaterootdir").DIRECTORY_SEPARATOR.$this->sTemplateName;
$this->xmlFile = $this->path.DIRECTORY_SEPARATOR.'config.xml';
}

}
else
{
$this->xmlFile = $this->path.DIRECTORY_SEPARATOR.'config.xml';
}

// We load the config file
// $this->config = simplexml_load_file(realpath ($this->xmlFile));
// Simple Xml is buggy on PHP < 5.4. The json_encode/decode workaround seems to be the most used one.

//////////////////////
// Config file loading

$bOldEntityLoaderState = libxml_disable_entity_loader(true); // @see: http://phpsecurity.readthedocs.io/en/latest/Injection-Attacks.html#xml-external-entity-injection
$sXMLConfigFile = file_get_contents( realpath ($this->xmlFile)); // @see: Now that entity loader is disabled, we can't use simplexml_load_file; so we must read the file with file_get_contents and convert it as a string

// Simple Xml is buggy on PHP < 5.4. The [ array -> json_encode -> json_decode ] workaround seems to be the most used one.
// @see: http://php.net/manual/de/book.simplexml.php#105330 (top comment on PHP doc for simplexml)
$this->config = json_decode( json_encode ( ( array ) simplexml_load_file(realpath ($this->xmlFile)), 1));
// Template configuration.
$this->viewPath = $this->path.DIRECTORY_SEPARATOR.$this->config->engine->pstpldirectory.DIRECTORY_SEPARATOR;
$this->siteLogo = (isset($this->config->files->logo))?$this->config->files->logo->filename:'';

// condition for user's template prior to 160219 (before this build, this configuration field wasn't present in the config.xml)
$this->filesPath = (isset($this->config->engine->filesdirectory))? $this->path.DIRECTORY_SEPARATOR.$this->config->engine->filesdirectory.DIRECTORY_SEPARATOR : $this->path . '/files/';
// condition for user's template prior to 160504
$this->overwrite_question_views = (isset($this->config->engine->overwrite_question_views))? ( $this->config->engine->overwrite_question_views=='true' || $this->config->engine->overwrite_question_views=='yes' ) : false;

$this->cssFramework = $this->config->engine->cssframework;
$oPackages = $this->config->engine->packages->package;
$this->packages = (array) $oPackages;
$this->otherFiles = $this->setOtherFiles();
$this->depends = $this->packages;
//$this->depends[] = (string) $this->cssFramework; // Bootstrap CSS is no more needed for Bootstrap templates (their custom css like "flat_and_modern.css" is a custom version of bootstrap.css )
$this->config = json_decode( json_encode ( ( array ) simplexml_load_string($sXMLConfigFile), 1));

// Template configuration
// Ternary operators test if configuration entry exists in the config file (to avoid PHP notice in user custom templates)
$this->viewPath = (isset($this->config->engine->pstpldirectory)) ? $this->path.DIRECTORY_SEPARATOR.$this->config->engine->pstpldirectory.DIRECTORY_SEPARATOR : $this->path;
$this->siteLogo = (isset($this->config->files->logo)) ? $this->config->files->logo->filename : '';
$this->filesPath = (isset($this->config->engine->filesdirectory)) ? $this->path.DIRECTORY_SEPARATOR.$this->config->engine->filesdirectory.DIRECTORY_SEPARATOR : $this->path . '/files/';
$this->cssFramework = (isset($this->config->engine->cssframework)) ? $this->config->engine->cssframework : '';
$this->packages = (isset($this->config->engine->packages->package)) ? $this->config->engine->packages->package : array();

// overwrite_question_views accept different values : "true" or "yes"
$this->overwrite_question_views = (isset($this->config->engine->overwrite_question_views)) ? ($this->config->engine->overwrite_question_views=='true' || $this->config->engine->overwrite_question_views=='yes' ) : false;

$this->otherFiles = $this->setOtherFiles();
$this->depends = $this->packages; // TODO: remove

// Package creation
$this->createTemplatePackage();

libxml_disable_entity_loader($bOldEntityLoaderState); // Put back entity loader to its original state, to avoid contagion to other applications on the server
return $this;
}

Expand Down

0 comments on commit fe57ea3

Please sign in to comment.