Merge pull request #29 from silverstripe-security/patch/3.1/SS-2016-010

[SS-2016-010] Prevent readonly fields loading submitted data

forms/Form.php

@@ -319,8 +319,21 @@ public function httpSubmission($request) {
 			$vars = $request->requestVars();
 		}
 
+		// construct an array of allowed fields that can be populated from request data.
+		// readonly or disabled fields should not be loading data from requests
+		$allowedFields = array();
+		$dataFields = $this->Fields()->dataFields();
+		if ($dataFields) {
+			/** @var FormField $field */
+			foreach ($this->Fields()->dataFields() as $name => $field) {
+				if (!$field->isReadonly() && !$field->isDisabled()) {
+					$allowedFields[] = $name;
+				}
+			}
+		}
+
 		// Populate the form
-		$this->loadDataFrom($vars, true);
+		$this->loadDataFrom($vars, true, $allowedFields);
 
 		// Protection against CSRF attacks
 		$token = $this->getSecurityToken();

forms/FormField.php

@@ -157,6 +157,14 @@ class FormField extends RequestHandler {
 	 */
 	protected $attributes = array();
 
+	/**
+	 * @config
+	 * @var array
+	 */
+	private static $casting = array(
+		'Value' => 'Text',
+	);
+
 	/**
 	 * Takes a field name and converts camelcase to spaced words. Also resolves combined field
 	 * names with dot syntax to spaced words.

forms/HtmlEditorField.php

@@ -26,6 +26,14 @@ class HtmlEditorField extends TextareaField {
 	 */
 	private static $sanitise_server_side = false;
 
+	/**
+	 * @config
+	 * @var array
+	 */
+	private static $casting = array(
+		'Value' => 'HTMLText',
+	);
+
 	protected $rows = 30;
 
 	/**

forms/ReadonlyField.php

@@ -3,7 +3,7 @@
  * Read-only field to display a non-editable value with a label.
  * Consider using an {@link LabelField} if you just need a label-less
  * value display.
- * 
+ *
  * @package forms
  * @subpackage fields-basic
  */
@@ -19,13 +19,13 @@ class ReadonlyField extends FormField {
 
 	/**
 	 * If true, a hidden field will be included in the HTML for the readonly field.
-	 * 
+	 *
 	 * This can be useful if you need to pass the data through on the form submission, as
 	 * long as it's okay than an attacker could change the data before it's submitted.
 	 *
 	 * This is disabled by default as it can introduce security holes if the data is not
 	 * allowed to be modified by the user.
-	 * 
+	 *
 	 * @param boolean $includeHiddenField
 	 */
 	public function setIncludeHiddenField($includeHiddenField) {
@@ -49,10 +49,28 @@ public function Field($properties = array()) {
 	}
 
 	public function Value() {
-		if($this->value) return $this->dontEscape ? $this->value : Convert::raw2xml($this->value);
+		if($this->value) return $this->value;
 		else return '<i>(' . _t('FormField.NONE', 'none') . ')</i>';
 	}
 
+	/**
+	 * This is a legacy fix to ensure that the `dontEscape` flag has an impact on readonly fields
+	 * now that we've moved to casting template values more rigidly
+	 *
+	 * @param string $field
+	 * @return string
+	 */
+	public function castingHelper($field) {
+		if (
+			(strcasecmp($field, 'Value') === 0)
+			&& ($this->dontEscape || empty($this->value))
+		) {
+			// Value is either empty, or unescaped
+			return 'HTMLText';
+		}
+		return parent::castingHelper($field);
+	}
+
 	public function getAttributes() {
 		return array_merge(
 			parent::getAttributes(),

forms/TextareaField.php

@@ -18,6 +18,11 @@
  * @subpackage fields-basic
  */
 class TextareaField extends FormField {
+
+	private static $casting = array(
+		'Value' => 'HTMLText',
+	);
+
 	/**
 	 * Visible number of text lines.
 	 *