[ss-2015-026]: BUG Fix FormField error messages not being encoded safely

docs/en/04_Changelogs/3.1.16.md

@@ -0,0 +1,25 @@
+# 3.1.16
+
+## Upgrading
+
+FormField validation messages generated by the `Validator` class will now be automatically XML
+encoded before being rendered alongside an invalid field.
+
+If a validation message in a custom `Validator` instance should be rendered as literal HTML,
+then the $message parameter for `Validator::validationError` should be passed as an instance
+of `HTMLText`
+
+For example:
+
+
+	:::php
+	class MyCustomValidator extends Validator {
+		public function php($data) { 
+			$this->validationError(
+				'EmailAddress',
+				DBField::create_field('HTMLText', "Invalid email. Please sign up at <a href='signup'>this page</a>")
+			);
+		}
+	}
+
+

forms/Form.php

@@ -1211,6 +1211,18 @@ class Form extends RequestHandler {
 			if($errors){
 				// Load errors into session and post back
 				$data = $this->getData();
+				// Encode validation messages as XML before saving into session state
+				// As per Form::addErrorMessage()
+				$errors = array_map(function($error) {
+					// Encode message as XML by default
+					if($error['message'] instanceof DBField) {
+						$error['message'] = $error['message']->forTemplate();;
+					} else {
+						$error['message'] = Convert::raw2xml($error['message']);
+					}
+					return $error;
+				}, $errors);
+
 				Session::set("FormInfo.{$this->FormName()}.errors", $errors);
 				Session::set("FormInfo.{$this->FormName()}.data", $data);
 				return false;

tests/forms/FormTest.php

@@ -226,6 +226,7 @@ class FormTest extends FunctionalTest {
 			'FormTest_Controller/Form',
 			array(
 				'Email' => 'invalid',
+				'Number' => '<a href="http://mysite.com">link</a>' // XSS attempt
 				// leaving out "Required" field
 			)
 		);
@@ -244,6 +245,16 @@ class FormTest extends FunctionalTest {
 			'Required fields show a notification on field when left blank'
 		);
 
+		$this->assertContains(
+			'&#039;&lt;a href=&quot;http://mysite.com&quot;&gt;link&lt;/a&gt;&#039; is not a number, only numbers can be accepted for this field',
+			$response->getBody(),
+			"Validation messages are safely XML encoded"
+		);
+		$this->assertNotContains(
+			'<a href="http://mysite.com">link</a>',
+			$response->getBody(),
+			"Unsafe content is not emitted directly inside the response body"
+		);
 	}
 	
 	public function testSessionSuccessMessage() {
@@ -630,7 +641,8 @@ class FormTest_Controller extends Controller implements TestOnly {
 			new FieldList(
 				new EmailField('Email'),
 				new TextField('SomeRequiredField'),
-				new CheckboxSetField('Boxes', null, array('1'=>'one','2'=>'two'))
+				new CheckboxSetField('Boxes', null, array('1'=>'one','2'=>'two')),
+				new NumericField('Number')
 			),
 			new FieldList(
 				new FormAction('doSubmit')