Merge pull request #2452 from chillu/pulls/escape-3.0

Escaping 3.0

docs/en/changelogs/3.0.7.md

@@ -0,0 +1,17 @@
+# 3.0.7
+
+## Overview
+
+### Security: XSS in form validation errors (SS-2013-008)
+
+See [announcement](http://www.silverstripe.org/ss-2013-008-xss-in-numericfield-validation/)
+
+### Security: XSS in CMS "Pages" section (SS-2013-009)
+
+See [announcement](http://www.silverstripe.org/ss-2013-009-xss-in-cms-pages-section/)
+
+### API: Form validation message no longer allow HTML
+
+Due to cross-site scripting concerns when user data is used for form messages,
+it is no longer possible to use HTML in `Form->sessionMessage()`, and consequently
+in the `FormField->validate()` API.

forms/Form.php

@@ -65,7 +65,7 @@ class Form extends RequestHandler {
 	protected $validator;
 	
 	protected $formMethod = "post";
-	
+
 	protected static $current_action;
 	
 	/**
@@ -144,6 +144,10 @@ class Form extends RequestHandler {
 	 */
 	protected $attributes = array();
 
+	public static $casting = array(
+		'Message' => 'Text'
+	);
+
 	/**
 	 * Create a new form, with the given fields an action buttons.
 	 * 
@@ -203,7 +207,7 @@ class Form extends RequestHandler {
 		'GET ' => 'httpSubmission',
 		'HEAD ' => 'httpSubmission',
 	);
-	
+
 	/**
 	 * Set up current form errors in session to
 	 * the current form if appropriate.
@@ -239,7 +243,7 @@ class Form extends RequestHandler {
 	 * if the form is valid.
 	 */
 	public function httpSubmission($request) {
-		$vars = $request->requestVars();
+			$vars = $request->requestVars();
 		if(isset($funcName)) {
 			Form::set_current_action($funcName);
 		}
@@ -281,7 +285,7 @@ class Form extends RequestHandler {
 		if(isset($funcName)) {
 			$this->setButtonClicked($funcName);
 		}
-		
+
 		// Permission checks (first on controller, then falling back to form)
 		if(
 			// Ensure that the action is actually a button or method on the form,
@@ -355,8 +359,8 @@ class Form extends RequestHandler {
 				}
 				return $this->controller->redirectBack();
 			}
-		}
-		
+	}
+	
 		// First, try a handler method on the controller (has been checked for allowed_actions above already)
 		if($this->controller->hasMethod($funcName)) {
 			return $this->controller->$funcName($vars, $this, $request);
@@ -439,7 +443,7 @@ class Form extends RequestHandler {
 	}
 
 	/**
-	 * Add an error message to a field on this form.  It will be saved into the session
+	 * Add a plain text error message to a field on this form.  It will be saved into the session
 	 * and used the next time this form is displayed.
 	 */
 	public function addErrorMessage($fieldName, $message, $messageType) {
@@ -865,7 +869,7 @@ class Form extends RequestHandler {
 		$this->formMethod = strtolower($method);
 		return $this;
 	}
-	
+
 	/**
 	 * Return the form's action attribute.
 	 * This is build by adding an executeForm get variable to the parent controller's Link() value

forms/FormField.php

@@ -93,6 +93,10 @@ class FormField extends RequestHandler {
 	 */
 	protected $attributes = array();
 
+	public static $casting = array(
+		'Message' => 'Text'
+	);
+
 	/**
 	 * Takes a fieldname and converts camelcase to spaced
 	 * words. Also resolves combined fieldnames with dot syntax

forms/TreeDropdownField.php

@@ -263,14 +263,28 @@ class TreeDropdownField extends FormField {
 				$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
+			);
+		};
 
 		if($isSubTree) {
-			return substr(trim($obj->getChildrenAsUL('', $eval, null, true, $this->childrenMethod)), 4, -5);
+			return substr(trim($obj->getChildrenAsUL('', $titleFn, null, true, $this->childrenMethod)), 4, -5);
 		} else {
-			return $obj->getChildrenAsUL('class="tree"', $eval, null, true, $this->childrenMethod);
+			return $obj->getChildrenAsUL('class="tree"', $titleFn, null, true, $this->childrenMethod);
 		}
 	}
 
@@ -290,7 +304,7 @@ class TreeDropdownField extends FormField {
 		
 		return true;
 	}
-	
+
 	/**
 	 * Populate $this->searchIds with the IDs of the pages matching the searched parameter and their parents.
 	 * Reverse-constructs the tree starting from the leaves. Initially taken from CMSSiteTreeFilter, but modified