From 808d6875cb397ad5194423a5d44939ee61d6598a Mon Sep 17 00:00:00 2001
From: Andrew O'Neil <andy@andyofniall.net>
Date: Sun, 28 Oct 2007 21:44:38 +0000
Subject: [PATCH] #941 - Security flaw: SS prone to CSRF attack

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@43901 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/control/Controller.php |  2 +-
 forms/Form.php              |  6 ++---
 search/SearchForm.php       |  2 ++
 security/LoginForm.php      | 54 ++++++++++++++++++-------------------
 4 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/core/control/Controller.php b/core/control/Controller.php
index 89061d373..4ea4882ae 100644
--- a/core/control/Controller.php
+++ b/core/control/Controller.php
@@ -157,7 +157,7 @@ class Controller extends ViewableData {
 			}
 			
 			// Protection against CSRF attacks
-			if($form->securityEnabled()) {
+			if($form->securityTokenEnabled()) {
 				$securityID = Session::get('SecurityID');
 
 				if(!$securityID || !isset($this->requestParams['SecurityID']) || $securityID != $this->requestParams['SecurityID']) {
diff --git a/forms/Form.php b/forms/Form.php
index f1ca05cc0..e16900783 100644
--- a/forms/Form.php
+++ b/forms/Form.php
@@ -174,7 +174,7 @@ class Form extends ViewableData {
 	 * @return FieldSet The form fields
 	 */
 	function Fields() {
-		if($this->securityEnabled()) {
+		if($this->securityTokenEnabled()) {
 			if(Session::get('SecurityID')) {
 				$securityID = Session::get('SecurityID');
 			} else {
@@ -686,7 +686,7 @@ class Form extends ViewableData {
 	 * against CSRF attacks, but you should disable this if you don't want to tie 
 	 * a form to a session - eg a search form.
 	 */
-	function disableSecurity() {
+	function disableSecurityToken() {
 		$this->security = false;
 	}
 	
@@ -696,7 +696,7 @@ class Form extends ViewableData {
 	 *
 	 * @return bool
 	 */
-	function securityEnabled() {
+	function securityTokenEnabled() {
 		return $this->security;
 	}
 
diff --git a/search/SearchForm.php b/search/SearchForm.php
index 2675a6aa7..047325388 100755
--- a/search/SearchForm.php
+++ b/search/SearchForm.php
@@ -36,6 +36,8 @@ class SearchForm extends Form {
 		$fields->push(new HiddenField("executeForm", null, $name));
 		
 		parent::__construct($controller, $name, $fields, $actions);
+		
+		$this->disableSecurityToken();
 	}
 	
 	function FormMethod() {
diff --git a/security/LoginForm.php b/security/LoginForm.php
index 4d28d06ba..6533f5725 100644
--- a/security/LoginForm.php
+++ b/security/LoginForm.php
@@ -17,35 +17,35 @@
  * @author Markus Lanthaler <markus@silverstripe.com>
  */
 abstract class LoginForm extends Form {
+	function __construct($controller, $name, $fields, $actions) {
+		parent::__construct($controller, $name, $fields, $actions);
+		
+		$this->disableSecurityToken();	
+	}
 
-  /**
-   * Authenticator class to use with this login form
-   *
-   * Set this variable to the authenticator class to use with this login
-   * form.
-   *
-   * @var string
-   */
-  protected $authenticator_class;
+	/**
+	 * Authenticator class to use with this login form
+	 * 
+	 * Set this variable to the authenticator class to use with this login
+	 * form.
+	 * @var string
+	 */
+	
+	protected $authenticator_class;
 
-
-  /**
-   * Get the authenticator class
-   *
-   * @return Authenticator Returns the authenticator class for this login
-   *                       form.
-   */
-  public function getAuthenticator() {
-    if(!class_exists($this->authenticator_class) ||
-       !is_subclass_of($this->authenticator_class, 'Authenticator')) {
-      user_error('The form uses an invalid authenticator class!',
-                 E_USER_ERROR);
-      return;
-    }
-
-    return new $this->authenticator_class;
-  }
+	/**
+	 * Get the authenticator class
+	 * @return Authenticator Returns the authenticator class for this login form.
+	 */
+	
+	public function getAuthenticator() {
+		if(!class_exists($this->authenticator_class) || !is_subclass_of($this->authenticator_class, 'Authenticator')) {
+			user_error('The form uses an invalid authenticator class!', E_USER_ERROR);
+			return;
+		}
+		
+		return new $this->authenticator_class;
+	}
 }
 
-
 ?>
\ No newline at end of file