tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

FIX Opt-out pf form message escaping (fixes #2796)

Browse Source 
This fixes a limitation introduced through http://www.silverstripe.org/ss-2013-008-xss-in-numericfield-validation/.
Form messages used to accept HTML, now they’re escaped by default, effectively removing the ability
to pass in HTML and take care of escaping manually.

We pass through HTML to message in core through the CTF system, so this needs to be fixed.
It’s an alternative fix to https://github.com/silverstripe/silverstripe-framework/pull/2803.
This commit is contained in:
Ingo Schommer 2014-01-31 10:38:34 +13:00 committed by Ingo Schommer
parent 0c2332f8bd
commit 0bae1826bb
6 changed files with 88 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
forms/ComplexTableField.php
View File

@ -642,7 +642,7 @@ JS;
try {
$childData->write();
} catch(ValidationException $e) {
$form->sessionMessage($e->getResult()->message(), 'bad');
$form->sessionMessage($e->getResult()->message(), 'bad', false);
return Director::redirectBack();
}
@ -670,16 +670,16 @@ JS;
_t('ComplexTableField.CLOSEPOPUP', 'Close Popup')
);
$editLink = Controller::join_links($this->Link(), 'item/' . $childData->ID . '/edit');
$editLink = Controller::join_links($this->Link(), 'item/' . (int)$childData->ID . '/edit');
$message = sprintf(
_t('ComplexTableField.SUCCESSADD', 'Added %s %s %s'),
$childData->singular_name(),
'<a href="' . $editLink . '">' . $childData->Title . '</a>',
'<a href="' . $editLink . '">' . Convert::raw2xml($childData->Title) . '</a>',
$closeLink
);
$form->sessionMessage($message, 'good');
$form->sessionMessage($message, 'good', false);
Director::redirectBack();
}
@ -851,7 +851,7 @@ class ComplexTableField_ItemRequest extends TableListField_ItemRequest {
$closeLink
);
$form->sessionMessage($message, 'good');
$form->sessionMessage($message, 'good', false);
Director::redirectBack();
}

34
forms/Form.php
View File

@ -137,10 +137,6 @@ class Form extends RequestHandler {
*/
protected $extraClasses = array();
public static $casting = array(
'Message' => 'Text'
);
/**
* Create a new form, with the given fields an action buttons.
*
@ -393,10 +389,10 @@ class Form extends RequestHandler {
* Add an error message to a field on this form. It will be saved into the session
* and used the next time this form is displayed.
*/
function addErrorMessage($fieldName, $message, $messageType) {
function addErrorMessage($fieldName, $message, $messageType, $escapeHtml = true) {
Session::add_to_array("FormInfo.{$this->FormName()}.errors", array(
'fieldName' => $fieldName,
'message' => $message,
'message' => $escapeHtml ? Convert::raw2xml($message) : $message,
'messageType' => $messageType,
));
}
@ -799,6 +795,7 @@ class Form extends RequestHandler {
$this->getMessageFromSession();
$message = $this->message;
$this->clearMessage();
return $message;
}
@ -816,7 +813,6 @@ class Form extends RequestHandler {
}else{
$this->message = Session::get("FormInfo.{$this->FormName()}.formError.message");
$this->messageType = Session::get("FormInfo.{$this->FormName()}.formError.type");
Session::clear("FormInfo.{$this->FormName()}");
}
}
@ -826,9 +822,12 @@ class Form extends RequestHandler {
*
* @param message the text of the message
* @param type Should be set to good, bad, or warning.
* @param boolean $escapeHtml Automatically sanitize the message. Set to FALSE if the message contains HTML.
* In that case, you might want to use {@link Convert::raw2xml()} to escape any
* user supplied data in the message.
*/
function setMessage($message, $type) {
$this->message = $message;
function setMessage($message, $type, $escapeHtml = true) {
$this->message = ($escapeHtml) ? Convert::raw2xml($message) : $message;
$this->messageType = $type;
}
@ -837,14 +836,23 @@ class Form extends RequestHandler {
*
* @param message the text of the message
* @param type Should be set to good, bad, or warning.
* @param boolean $escapeHtml Automatically sanitize the message. Set to FALSE if the message contains HTML.
* In that case, you might want to use {@link Convert::raw2xml()} to escape any
* user supplied data in the message.
*/
function sessionMessage($message, $type) {
Session::set("FormInfo.{$this->FormName()}.formError.message", $message);
function sessionMessage($message, $type, $escapeHtml = true) {
Session::set(
"FormInfo.{$this->FormName()}.formError.message",
$escapeHtml ? Convert::raw2xml($message) : $message
);
Session::set("FormInfo.{$this->FormName()}.formError.type", $type);
}
static function messageForForm( $formName, $message, $type ) {
Session::set("FormInfo.{$formName}.formError.message", $message);
static function messageForForm( $formName, $message, $type, $escapeHtml = true) {
Session::set(
"FormInfo.{$formName}.formError.message",
$escapeHtml ? Convert::raw2xml($message) : $message
);
Session::set("FormInfo.{$formName}.formError.type", $type);
}

11
forms/FormField.php
View File

@ -73,10 +73,6 @@ class FormField extends RequestHandler {
*/
protected $customValidationMessage = "";
public static $casting = array(
'Message' => 'Text'
);
/**
* Create a new field.
* @param name The internal field name, passed to forms.
@ -330,9 +326,12 @@ class FormField extends RequestHandler {
/**
* Sets the error message to be displayed on the form field
* Set by php validation of the form
* Set by php validation of the form.
*
* @param string $message Message to show to the user. Allows HTML content,
* which means you need to use Convert::raw2xml() for any user supplied data.
*/
function setError($message,$messageType){
function setError($message, $messageType){
$this->message = $message;
$this->messageType = $messageType;
}

10
security/MemberLoginForm.php
View File

@ -106,11 +106,17 @@ JS
* Get message from session
*/
protected function getMessageFromSession() {
parent::getMessageFromSession();
if(($member = Member::currentUser()) && !Session::get('MemberLoginForm.force_message')) {
$this->message = sprintf(_t('Member.LOGGEDINAS', "You're logged in as %s."), $member->{$this->loggedInAsField});
$this->sessionMessage(
sprintf(_t('Member.LOGGEDINAS', "You're logged in as %s."), $member->{$this->loggedInAsField}),
'notice'
);
}
Session::set('MemberLoginForm.force_message', false);
parent::getMessageFromSession();
return $this->message;
}

48
tests/forms/FormTest.php
View File

@ -325,9 +325,55 @@ class FormTest extends FunctionalTest {
SecurityToken::disable(); // restore original
}
function testMessageEscapeHtml() {
$form = $this->getStubForm();
$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
$form->sessionMessage('<em>Escaped HTML</em>', 'good', true);
$parser = new CSSContentParser($form->forTemplate());
$messageEls = $parser->getBySelector('.message');
$this->assertContains(
'&lt;em&gt;Escaped HTML&lt;/em&gt;',
$messageEls[0]->asXML()
);
$form = $this->getStubForm();
$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
$form->sessionMessage('<em>Unescaped HTML</em>', 'good', false);
$parser = new CSSContentParser($form->forTemplate());
$messageEls = $parser->getBySelector('.message');
$this->assertContains(
'<em>Unescaped HTML</em>',
$messageEls[0]->asXML()
);
}
function testFieldMessageEscapeHtml() {
$form = $this->getStubForm();
$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
$form->addErrorMessage('key1', '<em>Escaped HTML</em>', 'good', true);
$form->setupFormErrors();
$parser = new CSSContentParser($form->forTemplate());
$messageEls = $parser->getBySelector('#key1 .message');
$this->assertContains(
'&lt;em&gt;Escaped HTML&lt;/em&gt;',
$messageEls[0]->asXML()
);
$form = $this->getStubForm();
$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
$form->addErrorMessage('key1', '<em>Unescaped HTML</em>', 'good', false);
$form->setupFormErrors();
$parser = new CSSContentParser($form->forTemplate());
$messageEls = $parser->getBySelector('#key1 .message');
$this->assertContains(
'<em>Unescaped HTML</em>',
$messageEls[0]->asXML()
);
}
protected function getStubForm() {
return new Form(
new Controller(),
new FormTest_Controller(),
'Form',
new FieldSet(new TextField('key1')),
new FieldSet()

4
tests/security/SecurityTest.php
View File

@ -231,7 +231,7 @@ class SecurityTest extends FunctionalTest {
/* THE FIRST 4 TIMES, THE MEMBER SHOULDN'T BE LOCKED OUT */
if($i < 5) {
$this->assertNull($member->LockedOutUntil);
$this->assertContains($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED'));
$this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED')));
}
/* AFTER THAT THE USER IS LOCKED OUT FOR 15 MINUTES */
@ -272,7 +272,7 @@ class SecurityTest extends FunctionalTest {
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->assertNull($this->session()->inst_get('loggedInAs'));
$this->assertTrue(false !== stripos($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED')));
$this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED')));
$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$this->assertEquals($this->session()->inst_get('loggedInAs'), $member->ID);