From 245e0aae2f5f3eb0acba1d198ad8e196bb224462 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Wed, 11 Nov 2015 15:18:26 +1300
Subject: [PATCH] [ss-2015-026]: BUG Fix FormField error messages not being
 encoded safely

---
 docs/en/04_Changelogs/3.1.16.md | 25 +++++++++++++++++++++++++
 forms/Form.php                  | 12 ++++++++++++
 tests/forms/FormTest.php        | 16 ++++++++++++++--
 3 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 docs/en/04_Changelogs/3.1.16.md

diff --git a/docs/en/04_Changelogs/3.1.16.md b/docs/en/04_Changelogs/3.1.16.md
new file mode 100644
index 000000000..14ce134c9
--- /dev/null
+++ b/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>")
+			);
+		}
+	}
+
+
diff --git a/forms/Form.php b/forms/Form.php
index 3b0f3957b..769e6ece6 100644
--- a/forms/Form.php
+++ b/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;
diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php
index 7773c6725..c893979cf 100644
--- a/tests/forms/FormTest.php
+++ b/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
 			)
 		);
@@ -243,7 +244,17 @@ 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')