CVE-2014-2080: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site scripting (XSS) vulnerability in manager/templates/default/header.tpl in ModX Revolution before 2.2.11 allows remote attackers to inject arbitrary web script or HTML via the "a" parameter.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
Published: 2014-03-01T00:01Z
Repository: https://github.com/modxcms/revolution
Language: PHP
Files changed: core/docs/changelog.txt, core/model/modx/modmanagerrequest.class.php, manager/templates/default/header.tpl
Commit date: 2013-11-23 09:02:19 -0700
Commit message: Prevent XSS on actionVar in header.tpl

The smarty template was reflecting the $_GET['a'] parameter without converting htmlspecialchars (was only using strip_tags)

Diff: diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt
index 3771d9100..806682b81 100644
--- a/core/docs/changelog.txt
+++ b/core/docs/changelog.txt
@@ -1,9 +1,10 @@
 
 This file shows the changes in recent releases of MODX. The most current release is usually the
 development release, and is only shown to give an idea of what's currently in the pipeline.
 
+- Prevent XSS on actionVar in header.tpl
 - Fix caching of manager menus
 
 MODX Revolution 2.2.10-pl (October 7, 2013)
 ====================================
 - Increase modTransportPackage version columns range to smallint
diff --git a/core/model/modx/modmanagerrequest.class.php b/core/model/modx/modmanagerrequest.class.php
index a5b78cdbd..ededa84e3 100644
--- a/core/model/modx/modmanagerrequest.class.php
+++ b/core/model/modx/modmanagerrequest.class.php
@@ -51,10 +51,12 @@ class modManagerRequest extends modRequest {
      *
      * @access public
      * @return boolean True if successful.
      */
     public function initialize() {
+        $this->sanitizeRequest();
+        
         if (!defined('MODX_INCLUDES_PATH')) {
             define('MODX_INCLUDES_PATH',$this->modx->getOption('manager_path').'includes/');
         }
 
         /* load smarty template engine */
@@ -115,11 +117,11 @@ class modManagerRequest extends modRequest {
         $this->loadErrorHandler();
 
         $this->modx->invokeEvent('OnHandleRequest');
 
         /* save page to manager object. allow custom actionVar choice for extending classes. */
-        $this->action = isset($_REQUEST[$this->actionVar]) ? $_REQUEST[$this->actionVar] : $this->defaultAction;
+        $this->action = isset($_REQUEST[$this->actionVar]) ? (integer)$_REQUEST[$this->actionVar] : $this->defaultAction;
 
         /* invoke OnManagerPageInit event */
         $this->modx->invokeEvent('OnManagerPageInit',array('action' => $this->action));
         $this->prepareResponse();
     }
diff --git a/manager/templates/default/header.tpl b/manager/templates/default/header.tpl
index 996b412fb..31219465b 100644
--- a/manager/templates/default/header.tpl
+++ b/manager/templates/default/header.tpl
@@ -22,12 +22,12 @@
 {else}
 <script src="{$_config.manager_url}assets/ext3/adapter/ext/ext-base.js" type="text/javascript"></script>
 <script src="{$_config.manager_url}assets/ext3/ext-all.js" type="text/javascript"></script>
 {/if}
 <script src="{$_config.manager_url}assets/modext/core/modx.js" type="text/javascript"></script>
-<script src="{$_config.connectors_url}lang.js.php?ctx=mgr&topic=topmenu,file,resource,{$_lang_topics}&action={$smarty.get.a|strip_tags}" type="text/javascript"></script>
-<script src="{$_config.connectors_url}layout/modx.config.js.php?action={$smarty.get.a|strip_tags}{if $_ctx}&wctx={$_ctx}{/if}" type="text/javascript"></script>
+<script src="{$_config.connectors_url}lang.js.php?ctx=mgr&topic=topmenu,file,resource,{$_lang_topics}&action={$smarty.get.a|htmlspecialchars}" type="text/javascript"></script>
+<script src="{$_config.connectors_url}layout/modx.config.js.php?action={$smarty.get.a|htmlspecialchars}{if $_ctx}&wctx={$_ctx}{/if}" type="text/javascript"></script>
 
 {if $_config.compress_js && $_config.compress_js_groups}
 <script src="{$_config.manager_url}min/index.php?g=coreJs1" type="text/javascript"></script>
 <script src="{$_config.manager_url}min/index.php?g=coreJs2" type="text/javascript"></script>
 <script src="{$_config.manager_url}min/index.php?g=coreJs3" type="text/javascript"></script>

Vulnerable code:         $this->action = isset($_REQUEST[$this->actionVar]) ? $_REQUEST[$this->actionVar] : $this->defaultAction;
<script src="{$_config.connectors_url}lang.js.php?ctx=mgr&topic=topmenu,file,resource,{$_lang_topics}&action={$smarty.get.a|strip_tags}" type="text/javascript"></script>
<script src="{$_config.connectors_url}layout/modx.config.js.php?action={$smarty.get.a|strip_tags}{if $_ctx}&wctx={$_ctx}{/if}" type="text/javascript"></script>
Fixed code: - Prevent XSS on actionVar in header.tpl
        $this->sanitizeRequest();
        
        $this->action = isset($_REQUEST[$this->actionVar]) ? (integer)$_REQUEST[$this->actionVar] : $this->defaultAction;
<script src="{$_config.connectors_url}lang.js.php?ctx=mgr&topic=topmenu,file,resource,{$_lang_topics}&action={$smarty.get.a|htmlspecialchars}" type="text/javascript"></script>
<script src="{$_config.connectors_url}layout/modx.config.js.php?action={$smarty.get.a|htmlspecialchars}{if $_ctx}&wctx={$_ctx}{/if}" type="text/javascript"></script>