[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

@@ -15,6 +15,7 @@
 use MODX\Revolution\modElement;
 use MODX\Revolution\Processors\Model\CreateProcessor;
 use MODX\Revolution\modTemplate;
+use MODX\Revolution\modTemplateVar;
 use MODX\Revolution\Validation\modValidator;
 
 /**
@@ -29,6 +30,16 @@
  /** @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 +48,7 @@
  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 +59,47 @@
  */
  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);
+
+ $isTV = $this->classKey === modTemplateVar::class;
+
+ if ($isTV) {
+ 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 = $isTV
+ ? $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 +114,6 @@
  }
  }
 
- $locked = (bool)$this->getProperty('locked', false);
- $this->object->set('locked', $locked);
-
  $this->setElementProperties();
  $this->validateElement();
 
@@ -90,21 +129,17 @@
  }
 
  /**
- * 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

@@ -15,6 +15,7 @@
 use MODX\Revolution\modElement;
 use MODX\Revolution\Processors\Model\UpdateProcessor;
 use MODX\Revolution\modTemplate;
+use MODX\Revolution\modTemplateVar;
 
 /**
  * Abstract class for Update Element processors. To be extended for each derivative element type.
@@ -29,6 +30,16 @@
  /** @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 +57,44 @@
  $this->object->set('locked', (bool)$locked);
  }
 
- /* make sure a name was specified */
- $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name';
- $name = $this->getProperty($nameField, '');
+ $isTV = $this->classKey === modTemplateVar::class;
+
+ if ($isTV) {
+ 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 = $isTV
+ ? $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 +111,7 @@
  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 +121,10 @@
 
  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 +136,13 @@
 
  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 @@
  $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 @@
  $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 @@
  $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'));