diff --git a/core/control/Controller.php b/core/control/Controller.php index 5624e1910..89061d373 100644 --- a/core/control/Controller.php +++ b/core/control/Controller.php @@ -155,6 +155,15 @@ class Controller extends ViewableData { if(!isset($funcName)) { user_error("No action button has been clicked in this form executon, and no default has been allowed", E_USER_ERROR); } + + // Protection against CSRF attacks + if($form->securityEnabled()) { + $securityID = Session::get('SecurityID'); + + if(!$securityID || !isset($this->requestParams['SecurityID']) || $securityID != $this->requestParams['SecurityID']) { + trigger_error("Security ID doesn't match, possible CRSF attack.", E_USER_ERROR); + } + } // First, try a handler method on the controller diff --git a/forms/Form.php b/forms/Form.php index 29ac56157..f1ca05cc0 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -174,7 +174,21 @@ class Form extends ViewableData { * @return FieldSet The form fields */ function Fields() { - return $this->fields; + if($this->securityEnabled()) { + if(Session::get('SecurityID')) { + $securityID = Session::get('SecurityID'); + } else { + $securityID = rand(); + Session::set('SecurityID', $securityID); + } + + $fieldsClone = clone $this->fields; + $fieldsClone->push(new HiddenField('SecurityID', 'SecurityID', $securityID)); + + return $fieldsClone; + } else { + return $this->fields; + } } /** @@ -664,6 +678,27 @@ class Form extends ViewableData { function disableDefaultAction() { $this->hasDefaultAction = false; } + + private $security = true; + + /** + * Disable the requirement of a SecurityID in the Form. This security protects + * 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() { + $this->security = false; + } + + /** + * Returns true if security is enabled - that is if the SecurityID + * should be included and checked on this form. + * + * @return bool + */ + function securityEnabled() { + return $this->security; + } /** * Returns the name of a field, if that's the only field that the current controller is interested in. @@ -689,6 +724,7 @@ class Form extends ViewableData { self::$current_action = $action; } + ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // TESTING HELPERS ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/javascript/ComplexTableField.js b/javascript/ComplexTableField.js index bac67c5c6..02284deed 100755 --- a/javascript/ComplexTableField.js +++ b/javascript/ComplexTableField.js @@ -109,6 +109,10 @@ ComplexTableField.prototype = { return false; } } + + if($('SecurityID')) { + popupLink = popupLink + '&SecurityID=' + $('SecurityID').value; + } GB_OpenerObj = this; // use same url to refresh the table after saving the popup, but use a generic rendering method