Fixes bunch of various XSS issues in the manager modxcms#14335

* upstream/pr/14335: XSS in the tree Fix modxcms#14105 Fix modxcms#14104 Fix modxcms#14103 Fix modxcms#14102 Enable remote avatars
ily23 · Feb 6, 2019 · 71f894e · 71f894e
2 parents 3a83ee3 + 5b090a6
commit 71f894e
Show file tree

Hide file tree

Showing 9 changed files with 33 additions and 31 deletions.
diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt
@@ -4,6 +4,7 @@ development release, and is only shown to give an idea of what's currently in th
 
 MODX Revolution 2.7.1-pl (TBD)
 ====================================
+- Fixes bunch of various XSS issues in the manager [#14335]
 - Fix issue with resource list preventing parents from working correctly [#14329]
 - Fixed issues with tab width and very long strings in the vertical tabs [#14317]
 - Refactored tag input renderer to fix rendering with empty options list [#14319]

diff --git a/core/model/modx/moduser.class.php b/core/model/modx/moduser.class.php
@@ -904,10 +904,10 @@ public function getProfilePhoto($width = 128, $height = 128) {
         $source = modMediaSource::getDefaultSource($this->xpdo, $this->xpdo->getOption('photo_profile_source'));
         $source->initialize();
 
-        $path = $source->getBasePath($this->Profile->photo) . $this->Profile->photo;
+        $path = $source->prepareSrcForThumb($this->Profile->photo);
 
         return $this->xpdo->getOption('connectors_url', null, MODX_CONNECTORS_URL)
-            . "system/phpthumb.php?zc=1&h={$height}&w={$width}&src={$path}";
+            . "system/phpthumb.php?" . http_build_query(array("zc" => 1, "h" => $height, "w" => $width, "src" => $path));
     }
 
     /**

diff --git a/manager/assets/modext/modx.jsgrps-min.js b/manager/assets/modext/modx.jsgrps-min.js
diff --git a/manager/assets/modext/util/utilities.js b/manager/assets/modext/util/utilities.js
@@ -1,7 +1,7 @@
 Ext.namespace('MODx.util.Progress');
 /**
  * A JSON Reader specific to MODExt
- * 
+ *
  * @class MODx.util.JSONReader
  * @extends Ext.util.JSONReader
  * @param {Object} config An object of configuration properties
@@ -20,7 +20,7 @@ Ext.extend(MODx.util.JSONReader,Ext.data.JsonReader);
 Ext.reg('modx-json-reader',MODx.util.JSONReader);
 
 /**
- * @class MODx.util.Progress 
+ * @class MODx.util.Progress
  */
 MODx.util.Progress = {
     id: 0
@@ -66,7 +66,7 @@ Ext.override(Ext.form.BasicForm,{
         nodeToRecurse = nodeToRecurse || this;
         nodeToRecurse.items.each(function(f){
             if (!f.getValue) return;
-            
+
             if(f.items){
                 this.clearDirty(f);
             } else if(f.originalValue != f.getValue()){
@@ -77,7 +77,7 @@ Ext.override(Ext.form.BasicForm,{
 });
 
 
-/** 
+/**
  * Static Textfield
  */
 MODx.StaticTextField = Ext.extend(Ext.form.TextField, {
@@ -91,7 +91,7 @@ MODx.StaticTextField = Ext.extend(Ext.form.TextField, {
 });
 Ext.reg('statictextfield',MODx.StaticTextField);
 
-/** 
+/**
  * Static Boolean
  */
 MODx.StaticBoolean = Ext.extend(Ext.form.TextField, {
@@ -103,7 +103,7 @@ MODx.StaticBoolean = Ext.extend(Ext.form.TextField, {
         MODx.StaticBoolean.superclass.onRender.apply(this, arguments);
         this.on('change',this.onChange,this);
     }
-    
+
     ,setValue: function(v) {
         if (v === 1) {
             this.addClass('green');
@@ -147,13 +147,13 @@ Ext.form.setCheckboxValues = function(form,id,mask) {
     while ((f = form.findField(id+n)) !== null) {
         f.setValue((mask & (1<<n))?'true':'false');
         n=n+1;
-    } 
+    }
 };
 
 Ext.form.getCheckboxMask = function(cbgroup) {
     var mask='';
     if (typeof(cbgroup) !== 'undefined') {
-        if ((typeof(cbgroup)==='string')) { 
+        if ((typeof(cbgroup)==='string')) {
             mask = cbgroup+'';
         } else {
             for(var i=0,len=cbgroup.length;i<len;i=i+1) {
@@ -218,7 +218,7 @@ Ext.form.HourField = function(id,name,v){
         ,editable: false
         ,value: v || 1
         ,transform: id
-    }); 
+    });
 };
 
 
@@ -229,7 +229,7 @@ Ext.override(Ext.tree.TreeNodeUI,{
         return className && (' '+el.dom.className+' ').indexOf(' '+className+' ') !== -1;
     }
     ,renderElements : function(n, a, targetNode, bulkRender){
-        
+
         this.indentMarkup = n.parentNode ? n.parentNode.ui.getChildIndent() : '';
 
         var cb = Ext.isBoolean(a.checked),
@@ -247,7 +247,7 @@ Ext.override(Ext.tree.TreeNodeUI,{
                     iconMarkup,
                     cb ? ('<input class="x-tree-node-cb" type="checkbox" ' + (a.checked ? 'checked="checked" />' : '/>')) : '',
                     '<a hidefocus="on" class="x-tree-node-anchor" href="',href,'" tabIndex="1" ',
-                    a.hrefTarget ? ' target="'+a.hrefTarget+'"' : "", '><span unselectable="on">',n.text,"</span></a></div>",
+                    a.hrefTarget ? ' target="'+a.hrefTarget+'"' : "", '><span unselectable="on">',Ext.util.Format.htmlEncode(n.text),"</span></a></div>",
                     '<ul class="x-tree-node-ct" style="display:none;"></ul>',
                     "</li>"].join('');
 
@@ -266,7 +266,7 @@ Ext.override(Ext.tree.TreeNodeUI,{
         var index = 3;
         if(cb){
             this.checkbox = cs[3];
-            
+
             this.checkbox.defaultChecked = this.checkbox.checked;
             index++;
         }
@@ -295,8 +295,8 @@ Ext.override(Ext.tree.TreeNodeUI,{
 
 
 /* allows for messages in JSON responses */
-Ext.override(Ext.form.Action.Submit,{         
-    handleResponse : function(response){        
+Ext.override(Ext.form.Action.Submit,{
+    handleResponse : function(response){
         var m = Ext.decode(response.responseText); /* shaun 7/11/07 */
         if (this.form.errorReader) {
             var rs = this.form.errorReader.read(response);
@@ -320,7 +320,7 @@ Ext.override(Ext.form.Action.Submit,{
 });
 
 /* QTips to form fields */
-Ext.form.Field.prototype.afterRender = Ext.form.Field.prototype.afterRender.createSequence(function() { 
+Ext.form.Field.prototype.afterRender = Ext.form.Field.prototype.afterRender.createSequence(function() {
     if (this.description) {
         Ext.QuickTips.register({
             target:  this.getEl()
@@ -350,7 +350,7 @@ Ext.applyIf(Ext.form.Field,{
         }
         wrapDiv = field.getEl().up('div.x-form-item');
         if(wrapDiv) {
-            label = wrapDiv.child('label');        
+            label = wrapDiv.child('label');
         }
         if(label){
             return label;
@@ -365,7 +365,7 @@ MODx.util.Clipboard = function() {
             text = encodeURIComponent(text);
             return text.replace(/%0A/g, "%0D%0A");
         }
-        
+
         ,copy: function(text){
             if (Ext.isIE) {
                 window.clipboardData.setData("Text", text);
@@ -375,10 +375,10 @@ MODx.util.Clipboard = function() {
                     var divholder = document.createElement('div');
                     divholder.id = flashcopier;
                     document.body.appendChild(divholder);
-                }                
-                document.getElementById(flashcopier).innerHTML = '';                
+                }
+                document.getElementById(flashcopier).innerHTML = '';
                 var divinfo = '<embed src="' + MODx.config.manager_url
-                    + 'assets/modext/_clipboard.swf" FlashVars="clipboard=' 
+                    + 'assets/modext/_clipboard.swf" FlashVars="clipboard='
                     + MODx.util.Clipboard.escape(text)
                     + '" width="0" height="0" type="application/x-shockwave-flash"></embed>';
                 document.getElementById(flashcopier).innerHTML = divinfo;
@@ -408,7 +408,7 @@ Ext.ns('Ext.ux.grid');if('function'!==typeof RegExp.escape){RegExp.escape=functi
  * Ext JS Library 0.30
  * Copyright(c) 2006-2009, Ext JS, LLC.
  * [email protected]
- * 
+ *
  * http://extjs.com/license
  */
 Ext.SwitchButton = Ext.extend(Ext.Component, {
@@ -489,7 +489,7 @@ Ext.SwitchButton = Ext.extend(Ext.Component, {
         }
         return item;
     },
-    
+
     onClick : function(e){
         var target = e.getTarget('td', 2);
         if(!this.disabled && target){

diff --git a/manager/assets/modext/widgets/core/modx.grid.settings.js b/manager/assets/modext/widgets/core/modx.grid.settings.js
@@ -2,7 +2,7 @@ MODx.grid.SettingsGrid = function(config) {
     config = config || {};
     this.exp = new Ext.grid.RowExpander({
         tpl : new Ext.Template(
-            '<p class="desc">{description_trans}</p>'
+            '<p class="desc">{description_trans:htmlEncode}</p>'
         )
     });
 
@@ -646,4 +646,4 @@ MODx.window.UpdateSetting = function(config) {
     MODx.window.UpdateSetting.superclass.constructor.call(this,config);
 };
 Ext.extend(MODx.window.UpdateSetting,MODx.Window);
-Ext.reg('modx-window-setting-update',MODx.window.UpdateSetting);
+Ext.reg('modx-window-setting-update',MODx.window.UpdateSetting);
diff --git a/manager/assets/modext/widgets/resource/modx.tree.resource.js b/manager/assets/modext/widgets/resource/modx.tree.resource.js
@@ -493,7 +493,7 @@ Ext.extend(MODx.tree.Resource,MODx.tree.Tree,{
                             ,'hide':{fn:function() {this.destroy();}}
                         }
                     });
-                    w.title += ': <span dir="ltr">' + w.record.pagetitle + ' ('+ w.record.id + ')</span>';
+                    w.title += ': <span dir="ltr">' + Ext.util.Format.htmlEncode(w.record.pagetitle) + ' ('+ w.record.id + ')</span>';
                     w.setValues(r.object);
                     w.show(e.target,function() {
                         Ext.isSafari ? w.setPosition(null,30) : w.center();

diff --git a/manager/assets/modext/widgets/system/modx.grid.manager.log.js b/manager/assets/modext/widgets/system/modx.grid.manager.log.js
@@ -39,6 +39,7 @@ MODx.grid.ManagerLog = function(config) {
             header: _('object')
             ,dataIndex: 'name'
             ,width: 300
+            ,renderer: Ext.util.Format.htmlEncode
         }]
         ,tbar: [{
             xtype: 'button'

diff --git a/manager/controllers/default/security/user/update.class.php b/manager/controllers/default/security/user/update.class.php
@@ -133,7 +133,7 @@ private function _parseCustomData(array $remoteData = array(),$path = '') {
             );
             if (is_array($value)) {
                 $field['iconCls'] = 'icon-folder';
-                $field['text'] = $key;
+                $field['text'] = htmlentities($key,ENT_QUOTES,$encoding);
                 $field['leaf'] = false;
                 $field['children'] = $this->_parseCustomData($value,$key);
             } else {
@@ -147,7 +147,7 @@ private function _parseCustomData(array $remoteData = array(),$path = '') {
                     $v = substr($v,0,30).'...';
                 }
                 $field['iconCls'] = 'icon-terminal';
-                $field['text'] = $key.' - <i>'.htmlentities($v,ENT_QUOTES,$encoding).'</i>';
+                $field['text'] = htmlentities($key,ENT_QUOTES,$encoding).' - <i>'.htmlentities($v,ENT_QUOTES,$encoding).'</i>';
                 $field['leaf'] = true;
                 $field['value'] = $value;
             }

diff --git a/manager/templates/default/header.tpl b/manager/templates/default/header.tpl
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <html xmlns="http://www.w3.org/1999/xhtml" dir="{$_config.manager_direction}" lang="{$_config.manager_lang_attribute}" xml:lang="{$_config.manager_lang_attribute}">
 <head>
-<title>{if $_pagetitle}{$_pagetitle} | {/if}{$_config.site_name|strip_tags|escape}</title>
+<title>{if $_pagetitle}{$_pagetitle|escape} | {/if}{$_config.site_name|strip_tags|escape}</title>
 <meta http-equiv="Content-Type" content="text/html; charset={$_config.modx_charset}" />
 <meta name="viewport" content="width=device-width, initial-scale=1" />