From 747d4f440244ff37ff03a06d2cfa82b2034d2439 Mon Sep 17 00:00:00 2001 From: Simon Gow Date: Thu, 20 Sep 2018 14:53:35 +1200 Subject: [PATCH] Prevent storing formdata to cookies. - XSS an be stored in a cookie and potentially abused in other ways, so to prevent this we need to use session instead. This requires the user to have a session with silverstripe, but this is better than saving potentially malicious content in cookies. (Also some cookies have limited length). @see https://github.com/silverstripe/silverstripe-comments/issues/263 --- src/Forms/CommentForm.php | 50 ++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/Forms/CommentForm.php b/src/Forms/CommentForm.php index edee7de..364a7fc 100644 --- a/src/Forms/CommentForm.php +++ b/src/Forms/CommentForm.php @@ -14,7 +14,6 @@ use SilverStripe\Forms\RequiredFields; use SilverStripe\Forms\TextareaField; use SilverStripe\Forms\TextField; use SilverStripe\Security\Member; -use SilverStripe\Control\Cookie; use SilverStripe\Core\Convert; use SilverStripe\Security\Security; use SilverStripe\Comments\Model\Comment; @@ -132,21 +131,28 @@ class CommentForm extends Form // Set it so the user gets redirected back down to the form upon form fail $this->setRedirectToFormOnValidationError(true); - // load any data from the cookies - if ($data = Cookie::get('CommentsForm_UserData')) { - $data = Convert::json2array($data); + // load any data from the session + $data = $this->getSessionData(); + if (is_array($data)) { - $this->loadDataFrom(array( - 'Name' => isset($data['Name']) ? $data['Name'] : '', - 'URL' => isset($data['URL']) ? $data['URL'] : '', - 'Email' => isset($data['Email']) ? $data['Email'] : '' - )); + // load user data from previous form request back into form. + if (array_key_exists('UserData', $data)) { + $formData = Convert::json2array($data['UserData']); - // allow previous value to fill if comment not stored in cookie (i.e. validation error) - $prevComment = Cookie::get('CommentsForm_Comment'); + $this->loadDataFrom(array( + 'Name' => isset($formData['Name']) ? $formData['Name'] : '', + 'URL' => isset($formData['URL']) ? $formData['URL'] : '', + 'Email' => isset($formData['Email']) ? $formData['Email'] : '' + )); + } - if ($prevComment && $prevComment != '') { - $this->loadDataFrom(array('Comment' => $prevComment)); + // allow previous value to fill if comment + if (array_key_exists('Comment', $data)) { + $prevComment = $data['Comment']; + + if ($prevComment && $prevComment != '') { + $this->loadDataFrom(array('Comment' => $prevComment)); + } } } } @@ -184,8 +190,10 @@ class CommentForm extends Form } // cache users data - Cookie::set('CommentsForm_UserData', Convert::raw2json($data)); - Cookie::set('CommentsForm_Comment', $data['Comment']); + $form->setSessionData(array( + 'UserData' => Convert::raw2json($data), + 'Comment' => $data['Comment'] + )); // extend hook to allow extensions. Also see onAfterPostComment $this->controller->extend('onBeforePostComment', $form); @@ -246,8 +254,16 @@ class CommentForm extends Form $this->getRequest()->getSession()->set('CommentsModerated', 1); } - // clear the users comment since it passed validation - Cookie::set('CommentsForm_Comment', false); + // clear the users comment since the comment was successful. + if($comment->exists()){ + // Remove the comment data as it's been saved already. + unset($data['Comment']); + } + + // cache users data (name, email, etc to prepopulate on other forms). + $form->setSessionData(array( + 'UserData' => Convert::raw2json($data), + )); // Find parent link if (!empty($data['ReturnURL'])) {