Merge pull request #920 from TheSerapher/issue-670

[IMPROVED] Lockout user on invalid pin/password

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');

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);

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
  *

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':

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;

sql/007_accounts_update.sql

@@ -0,0 +1 @@
+ALTER TABLE  `accounts` ADD  `failed_pins` INT NOT NULL AFTER  `failed_logins` ;