[3.x] Restrict tag usage and ensure proper escaping of element name, caption, and description fields

_build/data/transport.core.system_settings.php

@@ -470,6 +470,42 @@
   'area' => 'site',
   'editedon' => null,
 ], '', true, true);
+$settings['elements_caption_allowedattr'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_caption_allowedattr']->fromArray([
+  'key' => 'elements_caption_allowedattr',
+  'value' => 'href',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
+$settings['elements_caption_allowedtags'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_caption_allowedtags']->fromArray([
+  'key' => 'elements_caption_allowedtags',
+  'value' => 'a',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
+$settings['elements_description_allowedattr'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_description_allowedattr']->fromArray([
+  'key' => 'elements_description_allowedattr',
+  'value' => 'href,src,class',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
+$settings['elements_description_allowedtags'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_description_allowedtags']->fromArray([
+  'key' => 'elements_description_allowedtags',
+  'value' => 'div,p,ul,ol,li,img,span,br,strong,b,em,i,a',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
 $settings['emailsender'] = $xpdo->newObject(modSystemSetting::class);
 $settings['emailsender']->fromArray([
   'key' => 'emailsender',

core/lexicon/en/chunk.inc.php

@@ -26,7 +26,7 @@
 $_lang['chunk_err_nf'] = 'Chunk not found!';
 $_lang['chunk_err_nfs'] = 'Chunk not found with id: [[+id]]';
 $_lang['chunk_err_ns'] = 'Chunk not specified.';
-$_lang['chunk_err_ns_name'] = 'Please specify a name.';
+$_lang['chunk_err_ns_name'] = 'Please specify a name for this chunk.';
 $_lang['chunk_lock'] = 'Lock chunk for editing';
 $_lang['chunk_lock_desc'] = 'Only users with “edit_locked” permissions can edit this Chunk.';
 $_lang['chunk_name_desc'] = 'Place the content generated by this Chunk in a Resource, Template, or other Chunk using the following MODX tag: [[+tag]]';

core/lexicon/en/plugin.inc.php

@@ -24,7 +24,7 @@
 $_lang['plugin_err_duplicate'] = 'An error occurred while trying to duplicate the plugin.';
 $_lang['plugin_err_nf'] = 'Plugin not found!';
 $_lang['plugin_err_ns'] = 'Plugin not specified.';
-$_lang['plugin_err_ns_name'] = 'Please specify a name for the plugin.';
+$_lang['plugin_err_ns_name'] = 'Please specify a name for this plugin.';
 $_lang['plugin_err_remove'] = 'An error occurred while trying to delete the plugin.';
 $_lang['plugin_err_save'] = 'An error occurred while saving the plugin.';
 $_lang['plugin_event_err_duplicate'] = 'An error occurred while trying to duplicate the plugin events';

core/lexicon/en/setting.inc.php

@@ -240,6 +240,18 @@
 $_lang['setting_default_per_page'] = 'Default Per Page';
 $_lang['setting_default_per_page_desc'] = 'The default number of results to show in grids throughout the manager.';
 
+$_lang['setting_elements_caption_allowedattr'] = 'Element Captions: Allowed Attributes';
+$_lang['setting_elements_caption_allowedattr_desc'] = 'When adding an element caption, the html tag attribute(s) provided in this comma-separated list will be preserved. This currently only applies to template variables (TVs).';
+
+$_lang['setting_elements_caption_allowedtags'] = 'Element Captions: Allowed Tags';
+$_lang['setting_elements_caption_allowedtags_desc'] = 'When adding an element caption, the html tag(s) provided in this comma-separated list will be preserved. This currently only applies to template variables (TVs).';
+
+$_lang['setting_elements_description_allowedattr'] = 'Element Descriptions: Allowed Attributes';
+$_lang['setting_elements_description_allowedattr_desc'] = 'When adding an element description, the html tag attribute(s) provided in this comma-separated list will be preserved.';
+
+$_lang['setting_elements_description_allowedtags'] = 'Element Descriptions: Allowed Tags';
+$_lang['setting_elements_description_allowedtags_desc'] = 'When adding an element description, the html tag(s) provided in this comma-separated list will be preserved.';
+
 $_lang['setting_emailsender'] = 'Registration Email From Address';
 $_lang['setting_emailsender_desc'] = 'Here you can specify the email address used when sending Users their usernames and passwords.';
 $_lang['setting_emailsender_err'] = 'Please state the administration email address.';

core/lexicon/en/snippet.inc.php

@@ -23,7 +23,7 @@
 $_lang['snippet_err_locked'] = 'This snippet is locked for editing.';
 $_lang['snippet_err_nf'] = 'Snippet not found!';
 $_lang['snippet_err_ns'] = 'Snippet not specified.';
-$_lang['snippet_err_ns_name'] = 'Please specify a name for the snippet.';
+$_lang['snippet_err_ns_name'] = 'Please specify a name for this snippet.';
 $_lang['snippet_err_remove'] = 'An error occurred while trying to delete the snippet.';
 $_lang['snippet_err_save'] = 'An error occurred while saving the snippet.';
 $_lang['snippet_execonsave'] = 'Execute snippet after saving.';

core/lexicon/en/tv.inc.php

@@ -33,6 +33,7 @@
 $_lang['tv_err_nf'] = 'TV not found.';
 $_lang['tv_err_nfs'] = 'TV not found with key: [[+id]]';
 $_lang['tv_err_ns'] = 'TV not specified.';
+$_lang['tv_err_ns_name'] = 'Please specify a name for this TV.';
 $_lang['tv_err_reserved_name'] = 'A TV cannot have the same name as a Resource field.';
 $_lang['tv_err_save_access_permissions'] = 'An error occured while attempting to save TV access permissions.';
 $_lang['tv_err_save'] = 'An error occurred while saving the TV.';

core/src/Revolution/Processors/Element/Create.php

@@ -29,6 +29,16 @@ abstract class Create extends CreateProcessor
     /** @var modElement $object */
     public $object;
 
+    protected $elementNameField = 'name';
+
+    public function initialize()
+    {
+        if ($this->classKey === modTemplate::class) {
+            $this->elementNameField = 'templatename';
+        }
+        return parent::initialize();
+    }
+
     /**
      * Cleanup the process and send back the response
      *
@@ -37,9 +47,7 @@ abstract class Create extends CreateProcessor
     public function cleanup()
     {
         $this->clearCache();
-        $fields = ['id', 'description', 'locked', 'category'];
-        array_push($fields, ($this->classKey == modTemplate::class ? 'templatename' : 'name'));
-
+        $fields = ['id', $this->elementNameField, 'description', 'locked', 'category'];
         return $this->success('', $this->object->get($fields));
     }
 
@@ -50,14 +58,47 @@ public function cleanup()
      */
     public function beforeSave()
     {
-        $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name';
-        $name = $this->getProperty($nameField, '');
-
-        /* verify element with that name does not already exist */
-        if ($this->alreadyExists($name)) {
-            $this->addFieldError($nameField, $this->modx->lexicon($this->objectType . '_err_ae', [
-                'name' => $name,
-            ]));
+        $locked = (bool)$this->getProperty('locked', false);
+        $this->object->set('locked', $locked);
+
+        $elementClassName = array_pop(explode('\\', $this->classKey));
+
+        if ($elementClassName === 'modTemplateVar') {
+            if ($caption = trim($this->getProperty('caption', ''))) {
+                $caption = $this->modx->stripHtml(
+                    $caption,
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
+                $this->object->set('caption', $caption);
+            }
+        }
+
+        if ($description = trim($this->getProperty('description', ''))) {
+            $description = $elementClassName === 'modTemplateVar'
+                ? $this->modx->stripHtml(
+                    $description,
+                    $this->modx->getOption('elements_description_allowedtags'),
+                    $this->modx->getOption('elements_description_allowedattr')
+                )
+                : strip_tags($description)
+                ;
+            $this->object->set('description', $description);
+        }
+
+        /* verify element has a name and that name does not already exist */
+
+        $name = $this->getProperty($this->elementNameField, '');
+
+        if (empty($name)) {
+            $this->addFieldError($this->elementNameField, $this->modx->lexicon($this->objectType . '_err_ns_name'));
+        } else {
+            if ($this->alreadyExists($name)) {
+                $this->addFieldError(
+                    $this->elementNameField,
+                    $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name])
+                );
+            }
         }
 
         $category = $this->getProperty('category', 0);
@@ -72,9 +113,6 @@ public function beforeSave()
             }
         }
 
-        $locked = (bool)$this->getProperty('locked', false);
-        $this->object->set('locked', $locked);
-
         $this->setElementProperties();
         $this->validateElement();
 
@@ -90,21 +128,17 @@ public function beforeSave()
     }
 
     /**
-     * Check to see if a Chunk already exists with specified name
+     * Check to see if an Element with the specified name already exists
      *
      * @param string $name
      *
      * @return bool
      */
     public function alreadyExists($name)
     {
-        if ($this->classKey == modTemplate::class) {
-            $c = ['templatename' => $name];
-        } else {
-            $c = ['name' => $name];
-        }
-
-        return $this->modx->getCount($this->classKey, $c) > 0;
+        return $this->modx->getCount($this->classKey, [
+            $this->elementNameField => $name,
+        ]) > 0;
     }
 
     /**

core/src/Revolution/Processors/Element/Update.php

@@ -29,6 +29,16 @@ abstract class Update extends UpdateProcessor
     /** @var modElement $object */
     public $object;
 
+    protected $elementNameField = 'name';
+
+    public function initialize()
+    {
+        if ($this->classKey === modTemplate::class) {
+            $this->elementNameField = 'templatename';
+        }
+        return parent::initialize();
+    }
+
     public function beforeSet()
     {
         // Make sure the element isn't locked
@@ -46,18 +56,44 @@ public function beforeSave()
             $this->object->set('locked', (bool)$locked);
         }
 
-        /* make sure a name was specified */
-        $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name';
-        $name = $this->getProperty($nameField, '');
+        $elementClassName = array_pop(explode('\\', $this->classKey));
+
+        if ($elementClassName === 'modTemplateVar') {
+            if ($caption = trim($this->getProperty('caption', ''))) {
+                $caption = $this->modx->stripHtml(
+                    $caption,
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
+                $this->object->set('caption', $caption);
+            }
+        }
+
+        if ($description = trim($this->getProperty('description', ''))) {
+            $description = $elementClassName === 'modTemplateVar'
+                ? $this->modx->stripHtml(
+                    $description,
+                    $this->modx->getOption('elements_description_allowedtags'),
+                    $this->modx->getOption('elements_description_allowedattr')
+                )
+                : strip_tags($description)
+                ;
+            $this->object->set('description', $description);
+        }
+
+        /* verify element has a name and that name does not already exist */
+
+        $name = $this->getProperty($this->elementNameField, '');
 
         if (empty($name)) {
-            $this->addFieldError($nameField, $this->modx->lexicon($this->objectType . '_err_ns_name'));
-        } elseif ($this->alreadyExists($name)) {
-            /* if changing name, but new one already exists */
-            $this->modx->error->addField(
-                $nameField,
-                $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name])
-            );
+            $this->addFieldError($this->elementNameField, $this->modx->lexicon($this->objectType . '_err_ns_name'));
+        } else {
+            if ($this->alreadyExists($name)) {
+                $this->addFieldError(
+                    $this->elementNameField,
+                    $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name])
+                );
+            }
         }
 
         /* category */
@@ -74,7 +110,7 @@ public function beforeSave()
         if ($this->object->staticContentChanged()) {
             if (!$this->object->isStaticSourceMutable()) {
                 $this->addFieldError('static_file', $this->modx->lexicon('element_static_source_immutable'));
-            } elseif (!$this->object->isStaticSourceValidPath()) {
+            } else if (!$this->object->isStaticSourceValidPath()) {
                 $this->addFieldError('static_file', $this->modx->lexicon('element_static_source_protected_invalid'));
             }
         }
@@ -84,12 +120,10 @@ public function beforeSave()
 
     public function alreadyExists($name)
     {
-        $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name';
-
         return $this->modx->getCount($this->classKey, [
-                'id:!=' => $this->object->get('id'),
-                $nameField => $name,
-            ]) > 0;
+            'id:!=' => $this->object->get('id'),
+            $this->elementNameField => $name,
+        ]) > 0;
     }
 
     public function afterSave()
@@ -101,11 +135,13 @@ public function afterSave()
 
     public function cleanup()
     {
-        $fields = array('id', 'description', 'locked', 'category', 'content');
-        array_push($fields, ($this->classKey == modTemplate::class ? 'templatename' : 'name'));
+        $fields = ['id', $this->elementNameField, 'description', 'locked', 'category', 'content'];
         return $this->success(
             '',
-            array_merge($this->object->get($fields), ['previous_category' => $this->previousCategory])
+            array_merge(
+                $this->object->get($fields),
+                ['previous_category' => $this->previousCategory]
+            )
         );
     }
 }

core/src/Revolution/Processors/Security/Forms/Set/Update.php

@@ -1,4 +1,5 @@
 <?php
+
 /*
  * This file is part of MODX Revolution.
  *
@@ -121,6 +122,11 @@ public function setFieldRules()
                 $this->newRules[] = $rule;
             }
             if (!empty($field['label'])) {
+                $field['label'] = $this->modx->stripHtml(
+                    $field['label'],
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
                 $rule = $this->modx->newObject(modActionDom::class);
                 $rule->set('set', $this->object->get('id'));
                 $rule->set('action', $this->object->get('action'));
@@ -224,6 +230,7 @@ public function setTabRules()
                     $this->newRules[] = $rule;
                 }
                 if (!empty($tab['label'])) {
+                    $tab['label'] = strip_tags($tab['label']);
                     $rule = $this->modx->newObject(modActionDom::class);
                     $rule->set('set', $this->object->get('id'));
                     $rule->set('action', $this->object->get('action'));
@@ -285,6 +292,11 @@ public function setTVRules()
                 $this->newRules[] = $rule;
             }
             if (!empty($tvData['label'])) {
+                $tvData['label'] = $this->modx->stripHtml(
+                    $tvData['label'],
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
                 $rule = $this->modx->newObject(modActionDom::class);
                 $rule->set('set', $this->object->get('id'));
                 $rule->set('action', $this->object->get('action'));