Permalink
Browse files

FIX Auto-escape titles in TreeDropdownField

Related to SS-2013-009. While the default "TreeTitle" was escaped
within the SiteTree->TreeTitle() getter, other properties like SiteTree->Title
weren't escaped. The new logic uses the underlying casting helpers
on the processed objects.
  • Loading branch information...
1 parent 78ce99b commit f3ef04a432571a787722d840d99d6ca26750e28e @chillu chillu committed Sep 24, 2013
Showing with 24 additions and 6 deletions.
  1. +5 −1 docs/en/changelogs/rc/3.1.0-rc3.md
  2. +19 −5 forms/TreeDropdownField.php
@@ -4,4 +4,8 @@
### Security: XSS in CMS "Security" section (SS-2013-007)
-See [announcement](http://www.silverstripe.org/ss-2013-007-xss-in-cms-security-section/)
+See [announcement](http://www.silverstripe.org/ss-2013-007-xss-in-cms-security-section/)
+
+### Security: XSS in CMS "Pages" section (SS-2013-009)
+
+See [announcement](http://www.silverstripe.org/ss-2013-009-xss-in-cms-pages-section/)
@@ -265,9 +265,23 @@ public function tree(SS_HTTPRequest $request) {
$obj->markToExpose($this->objectForKey($value));
}
}
- $eval = '"<li id=\"selector-' . $this->getName() . '-{$child->' . $this->keyField . '}\" data-id=\"$child->'
- . $this->keyField . '\" class=\"class-$child->class"'
- . ' . $child->markingClasses() . "\"><a rel=\"$child->ID\">" . $child->' . $this->labelField . ' . "</a>"';
+
+ $self = $this;
+ $escapeLabelField = ($obj->escapeTypeForField($this->labelField) != 'xml');
+ $titleFn = function(&$child) use(&$self, $escapeLabelField) {
+ $keyField = $self->keyField;
+ $labelField = $self->labelField;
+ return sprintf(
+ '<li id="selector-%s-%s" data-id="%s" class="class-%s %s"><a rel="%d">%s</a>',
+ Convert::raw2xml($self->getName()),
+ Convert::raw2xml($child->$keyField),
+ Convert::raw2xml($child->$keyField),
+ Convert::raw2xml($child->class),
+ Convert::raw2xml($child->markingClasses()),
+ (int)$child->ID,
+ $escapeLabelField ? Convert::raw2xml($child->$labelField) : $child->$labelField
+ );
+ };
// Limit the amount of nodes shown for performance reasons.
// Skip the check if we're filtering the tree, since its not clear how many children will
@@ -290,7 +304,7 @@ public function tree(SS_HTTPRequest $request) {
if($isSubTree) {
$html = $obj->getChildrenAsUL(
"",
- $eval,
+ $titleFn,
null,
true,
$this->childrenMethod,
@@ -303,7 +317,7 @@ public function tree(SS_HTTPRequest $request) {
} else {
$html = $obj->getChildrenAsUL(
'class="tree"',
- $eval,
+ $titleFn,
null,
true,
$this->childrenMethod,

0 comments on commit f3ef04a

Please sign in to comment.