From 256b5f59c60f89be5da5d7c89e78a56d3a6876b3 Mon Sep 17 00:00:00 2001
From: Sebastian Grewe <sebastian@grewe.ca>
Date: Sat, 7 Dec 2013 22:14:14 +0100
Subject: [PATCH] [IMPROVED] Lockout user on invalid pin/password

This will lock a user account if a password or PIN has been entered
wrong for multiple times in a row. When unlocking the account via admin
panel, both counters are reset so the user can log in again.

This should fix issues with brute force attacks to access user accounts.

Please see configuration dist file for new config options.
Please import SQL upgrade 007 to add new column to user accounts table.

Addresses #670 and should be merged once tested.
---
 public/include/autoloader.inc.php         |  2 +-
 public/include/classes/user.class.php     | 35 ++++++++++++++++++-----
 public/include/config/global.inc.dist.php | 19 ++++++++++++
 public/include/pages/account/edit.inc.php |  2 +-
 public/include/pages/admin/user.inc.php   |  5 ++++
 sql/007_accounts_update.sql               |  1 +
 6 files changed, 55 insertions(+), 9 deletions(-)
 create mode 100644 sql/007_accounts_update.sql

diff --git a/public/include/autoloader.inc.php b/public/include/autoloader.inc.php
index 3520cc05..361a5f36 100644
--- a/public/include/autoloader.inc.php
+++ b/public/include/autoloader.inc.php
@@ -46,6 +46,7 @@ require_once(CLASS_DIR . '/statscache.class.php');
 require_once(CLASS_DIR . '/bitcoin.class.php');
 require_once(CLASS_DIR . '/bitcoinwrapper.class.php');
 require_once(CLASS_DIR . '/monitoring.class.php');
+require_once(CLASS_DIR . '/notification.class.php');
 require_once(CLASS_DIR . '/user.class.php');
 require_once(CLASS_DIR . '/invitation.class.php');
 require_once(CLASS_DIR . '/share.class.php');
@@ -53,7 +54,6 @@ require_once(CLASS_DIR . '/worker.class.php');
 require_once(CLASS_DIR . '/statistics.class.php');
 require_once(CLASS_DIR . '/transaction.class.php');
 require_once(CLASS_DIR . '/roundstats.class.php');
-require_once(CLASS_DIR . '/notification.class.php');
 require_once(CLASS_DIR . '/news.class.php');
 require_once(CLASS_DIR . '/api.class.php');
 require_once(INCLUDE_DIR . '/lib/Michelf/Markdown.php');
diff --git a/public/include/classes/user.class.php b/public/include/classes/user.class.php
index e68ae854..4c930a7f 100644
--- a/public/include/classes/user.class.php
+++ b/public/include/classes/user.class.php
@@ -46,6 +46,9 @@ class User extends Base {
   public function getUserFailed($id) {
    return $this->getSingle($id, 'failed_logins', 'id');
   }
+  public function getUserPinFailed($id) {
+   return $this->getSingle($id, 'failed_pins', 'id');
+  }
   public function isNoFee($id) {
     return $this->getUserNoFee($id);
   }
@@ -71,10 +74,18 @@ class User extends Base {
     $field = array( 'name' => 'failed_logins', 'type' => 'i', 'value' => $value);
     return $this->updateSingle($id, $field);
   }
+  public function setUserPinFailed($id, $value) {
+    $field = array( 'name' => 'failed_pins', 'type' => 'i', 'value' => $value);
+    return $this->updateSingle($id, $field);
+  }
   private function incUserFailed($id) {
     $field = array( 'name' => 'failed_logins', 'type' => 'i', 'value' => $this->getUserFailed($id) + 1);
     return $this->updateSingle($id, $field);
   }
+  private function incUserPinFailed($id) {
+    $field = array( 'name' => 'failed_pins', 'type' => 'i', 'value' => $this->getUserPinFailed($id) + 1);
+    return $this->updateSingle($id, $field);
+  }
   private function setUserIp($id, $ip) {
     $field = array( 'name' => 'loggedIp', 'type' => 's', 'value' => $ip );
     return $this->updateSingle($id, $field);
@@ -122,8 +133,12 @@ class User extends Base {
         return true;
     }
     $this->setErrorMessage("Invalid username or password");
-    if ($id = $this->getUserId($username))
+    if ($id = $this->getUserId($username)) {
       $this->incUserFailed($id);
+      // Check if this account should be locked
+      if (isset($this->config['maxfailed']['login']) && $this->getUserFailed($id) >= $this->config['maxfailed']['login'])
+        $this->changeLocked($id);
+    }
 
     return false;
   }
@@ -139,12 +154,17 @@ class User extends Base {
     $this->debug->append("Confirming PIN for $userId and pin $pin", 2);
     $stmt = $this->mysqli->prepare("SELECT pin FROM $this->table WHERE id=? AND pin=? LIMIT 1");
     $pin_hash = $this->getHash($pin);
-    $stmt->bind_param('is', $userId, $pin_hash);
-    $stmt->execute();
-    $stmt->bind_result($row_pin);
-    $stmt->fetch();
-    $stmt->close();
-    return $pin_hash === $row_pin;
+    if ($stmt->bind_param('is', $userId, $pin_hash) && $stmt->execute() && $stmt->bind_result($row_pin) && $stmt->fetch()) {
+      $this->setUserPinFailed($userId, 0);
+      return $pin_hash === $row_pin;
+    }
+    $this->incUserPinFailed($userId);
+    // Check if this account should be locked
+    if (isset($this->config['maxfailed']['pin']) && $this->getUserPinFailed($userId) >= $this->config['maxfailed']['pin']) {
+      $this->changeLocked($userId);
+      $this->logoutUser();
+    }
+    return false;
   }
 
   /**
@@ -641,3 +661,4 @@ $user->setMail($mail);
 $user->setToken($oToken);
 $user->setBitcoin($bitcoin);
 $user->setSetting($setting);
+$user->setErrorCodes($aErrorCodes);
diff --git a/public/include/config/global.inc.dist.php b/public/include/config/global.inc.dist.php
index 28a7bf96..2d1ba2a6 100644
--- a/public/include/config/global.inc.dist.php
+++ b/public/include/config/global.inc.dist.php
@@ -69,6 +69,25 @@ $config['wallet']['host'] = 'localhost:19334';
 $config['wallet']['username'] = 'testnet';
 $config['wallet']['password'] = 'testnet';
 
+/**
+ * Lock account after maximum failed logins
+ *
+ * Explanation:
+ *   To avoid accounts being hacked by brute force attacks,
+ *   set a maximum amount of failed login or pin entry attempts before locking
+ *   the account. They will need to contact site support to re-enable the account.
+ *
+ *   This also applies for invalid PIN entries, which is covered by the pin option.
+ *
+ *   Workers are not affected by this lockout, mining will continue as usual.
+ *
+ * Default:
+ *   login  =  3
+ *   pin    =  3
+ **/
+$config['maxfailed']['login'] = 3;
+$config['maxfailed']['pin'] = 3;
+
 /**
  * Getting Started Config
  *
diff --git a/public/include/pages/account/edit.inc.php b/public/include/pages/account/edit.inc.php
index 006f9862..547032b5 100644
--- a/public/include/pages/account/edit.inc.php
+++ b/public/include/pages/account/edit.inc.php
@@ -6,7 +6,7 @@ if (!defined('SECURITY'))
 
 if ($user->isAuthenticated()) {
   if ( ! $user->checkPin($_SESSION['USERDATA']['id'], @$_POST['authPin']) && @$_POST['do']) {
-    $_SESSION['POPUP'][] = array('CONTENT' => 'Invalid PIN','TYPE' => 'errormsg');
+    $_SESSION['POPUP'][] = array('CONTENT' => 'Invalid PIN. ' . ($config['maxfailed']['pin'] - $user->getUserPinFailed($_SESSION['USERDATA']['id'])) . ' attempts remaining.', 'TYPE' => 'errormsg');
   } else {
     switch (@$_POST['do']) {
     case 'cashOut':
diff --git a/public/include/pages/admin/user.inc.php b/public/include/pages/admin/user.inc.php
index 5ad323d5..c7c7470a 100644
--- a/public/include/pages/admin/user.inc.php
+++ b/public/include/pages/admin/user.inc.php
@@ -14,7 +14,12 @@ $aRoundShares = $statistics->getRoundShares();
 switch (@$_POST['do']) {
 case 'lock':
   $supress_master = 1;
+  // Reset user account
   $user->changeLocked($_POST['account_id']);
+  if ($user->isLocked($_POST['account_id']) == 0) {
+    $user->setUserFailed($_POST['account_id'], 0);
+    $user->setUserPinFailed($_POST['account_id'], 0);
+  }
   break;
 case 'fee':
   $supress_master = 1;
diff --git a/sql/007_accounts_update.sql b/sql/007_accounts_update.sql
new file mode 100644
index 00000000..ae83fe7b
--- /dev/null
+++ b/sql/007_accounts_update.sql
@@ -0,0 +1 @@
+ALTER TABLE  `accounts` ADD  `failed_pins` INT NOT NULL AFTER  `failed_logins` ;