From 504831fe4d5c21b1dafbd7e8cdde7607592c8fb7 Mon Sep 17 00:00:00 2001
From: Jared Hancock <jared@osticket.com>
Date: Wed, 11 Feb 2015 13:47:16 -0600
Subject: [PATCH] login: Require CSRF token to login
This patch fixes a vulnerable scenario, where sequential login attempts can
be made without an existing session, and without a valid CSRF token. This
scenario lends itself well for brute force password attempts, because
attackers can avoid using a session and still send requests to determine if
a set of credentials are valid. This vector also avoids the authentication
lockout mechanism, because it requires an ongoing session to shutdown the
requests.
This patch addresses the issue by requiring a session and a valid CSRF token
generated by the server and placed in the session to be submitted with the
credentials. Therefore, an existing session and a Cookie header are required
to process a login attempt. Secondly, the CSRF token will be changed on the
server after each login processed. Therefore, for each session, a subsequent
GET request would be necessary before submitting another login attempt.
---
include/class.csrf.php | 11 +++++++----
login.php | 14 ++++++++++++++
scp/login.php | 10 ++++++++++
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/include/class.csrf.php b/include/class.csrf.php
index 94f103cb8..a1c3aed21 100644
--- a/include/class.csrf.php
+++ b/include/class.csrf.php
@@ -53,12 +53,15 @@ Class CSRF {
return $this->name;
}
- function getToken() {
+ function rotate() {
+ $this->csrf['token'] = sha1(session_id().Crypto::random(16).SECRET_SALT);
+ $this->csrf['time'] = time();
+ }
- if(!$this->csrf['token'] || $this->isExpired()) {
+ function getToken() {
- $this->csrf['token'] = sha1(session_id().Crypto::random(16).SECRET_SALT);
- $this->csrf['time'] = time();
+ if (!$this->csrf['token'] || $this->isExpired()) {
+ $this->rotate();
} else {
//Reset the timer
$this->csrf['time'] = time();
diff --git a/login.php b/login.php
index f79bf8563..a90376ec3 100644
--- a/login.php
+++ b/login.php
@@ -31,6 +31,20 @@ else
$inc = 'login.inc.php';
$suggest_pwreset = false;
+
+// Check the CSRF token, and ensure that future requests will have to use a
+// different CSRF token. This will help ward off both parallel and serial
+// brute force attacks, because new tokens will have to be requested for
+// each attempt.
+if ($_POST) {
+ // Check CSRF token
+ if (!$ost->checkCSRFToken())
+ Http::response(400, __('Valid CSRF Token Required'));
+
+ // Rotate the CSRF token (original cannot be reused)
+ $ost->getCSRF()->rotate();
+}
+
if ($_POST && isset($_POST['luser'])) {
if (!$_POST['luser'])
$errors['err'] = __('Valid username or email address is required');
diff --git a/scp/login.php b/scp/login.php
index ad07a831b..bbf8dc7a3 100644
--- a/scp/login.php
+++ b/scp/login.php
@@ -31,6 +31,16 @@ $msg = $msg ?: ($content ? $content->getName() : __('Authentication Required'));
$dest=($dest && (!strstr($dest,'login.php') && !strstr($dest,'ajax.php')))?$dest:'index.php';
$show_reset = false;
if($_POST) {
+ // Check the CSRF token, and ensure that future requests will have to
+ // use a different CSRF token. This will help ward off both parallel and
+ // serial brute force attacks, because new tokens will have to be
+ // requested for each attempt.
+ if (!$ost->checkCSRFToken())
+ Http::response(400, __('Valid CSRF Token Required'));
+
+ // Rotate the CSRF token (original cannot be reused)
+ $ost->getCSRF()->rotate();
+
// Lookup support backends for this staff
$username = trim($_POST['userid']);
if ($user = StaffAuthenticationBackend::process($username,
--
GitLab