Fixes rate limiting bugs
parent
58633313e1
commit
4a18c344c8
|
@ -7,8 +7,8 @@ class RateLimiter {
|
||||||
public $decayMinutes = 15; // Time window in minutes
|
public $decayMinutes = 15; // Time window in minutes
|
||||||
public $autoBlacklistThreshold = 10; // Attempts before auto-blacklist
|
public $autoBlacklistThreshold = 10; // Attempts before auto-blacklist
|
||||||
public $autoBlacklistDuration = 24; // Hours to blacklist for
|
public $autoBlacklistDuration = 24; // Hours to blacklist for
|
||||||
public $authRatelimitTable = 'login_attempts';
|
public $authRatelimitTable = 'login_attempts'; // For username/password attempts
|
||||||
public $pagesRatelimitTable = 'pages_rate_limits';
|
public $pagesRatelimitTable = 'pages_rate_limits'; // For page requests
|
||||||
public $whitelistTable = 'ip_whitelist';
|
public $whitelistTable = 'ip_whitelist';
|
||||||
public $blacklistTable = 'ip_blacklist';
|
public $blacklistTable = 'ip_blacklist';
|
||||||
private $pageLimits = [
|
private $pageLimits = [
|
||||||
|
@ -33,7 +33,7 @@ class RateLimiter {
|
||||||
// Authentication attempts table
|
// Authentication attempts table
|
||||||
$sql = "CREATE TABLE IF NOT EXISTS {$this->authRatelimitTable} (
|
$sql = "CREATE TABLE IF NOT EXISTS {$this->authRatelimitTable} (
|
||||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||||
ip_address TEXT NOT NULL UNIQUE,
|
ip_address TEXT NOT NULL,
|
||||||
username TEXT NOT NULL,
|
username TEXT NOT NULL,
|
||||||
attempted_at TEXT DEFAULT (DATETIME('now'))
|
attempted_at TEXT DEFAULT (DATETIME('now'))
|
||||||
)";
|
)";
|
||||||
|
@ -157,19 +157,24 @@ class RateLimiter {
|
||||||
* Check if an IP is whitelisted
|
* Check if an IP is whitelisted
|
||||||
*/
|
*/
|
||||||
public function isIpWhitelisted($ip) {
|
public function isIpWhitelisted($ip) {
|
||||||
// Check exact IP match and CIDR ranges
|
// Check exact IP match first
|
||||||
$stmt = $this->db->prepare("SELECT ip_address, is_network FROM {$this->whitelistTable}");
|
$stmt = $this->db->prepare("SELECT ip_address FROM {$this->whitelistTable} WHERE ip_address = ?");
|
||||||
$stmt->execute();
|
$stmt->execute([$ip]);
|
||||||
|
$row = $stmt->fetch(PDO::FETCH_ASSOC);
|
||||||
|
if ($row) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
|
// Only check ranges for IPv4 addresses
|
||||||
if ($row['is_network']) {
|
if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
|
||||||
|
// Check network ranges
|
||||||
|
$stmt = $this->db->prepare("SELECT ip_address FROM {$this->whitelistTable} WHERE is_network = 1");
|
||||||
|
$stmt->execute();
|
||||||
|
|
||||||
|
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
|
||||||
if ($this->ipInRange($ip, $row['ip_address'])) {
|
if ($this->ipInRange($ip, $row['ip_address'])) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
} else {
|
|
||||||
if ($ip === $row['ip_address']) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -177,8 +182,18 @@ class RateLimiter {
|
||||||
}
|
}
|
||||||
|
|
||||||
private function ipInRange($ip, $cidr) {
|
private function ipInRange($ip, $cidr) {
|
||||||
|
// Only work with IPv4 addresses
|
||||||
|
if (!filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
list($subnet, $bits) = explode('/', $cidr);
|
list($subnet, $bits) = explode('/', $cidr);
|
||||||
|
|
||||||
|
// Make sure subnet is IPv4
|
||||||
|
if (!filter_var($subnet, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
$ip = ip2long($ip);
|
$ip = ip2long($ip);
|
||||||
$subnet = ip2long($subnet);
|
$subnet = ip2long($subnet);
|
||||||
$mask = -1 << (32 - $bits);
|
$mask = -1 << (32 - $bits);
|
||||||
|
@ -412,21 +427,12 @@ class RateLimiter {
|
||||||
// Record this attempt
|
// Record this attempt
|
||||||
$sql = "INSERT INTO {$this->authRatelimitTable} (ip_address, username) VALUES (:ip, :username)";
|
$sql = "INSERT INTO {$this->authRatelimitTable} (ip_address, username) VALUES (:ip, :username)";
|
||||||
$stmt = $this->db->prepare($sql);
|
$stmt = $this->db->prepare($sql);
|
||||||
$stmt->execute([
|
try {
|
||||||
':ip' => $ipAddress,
|
$stmt->execute([
|
||||||
':username' => $username
|
':ip' => $ipAddress,
|
||||||
]);
|
':username' => $username
|
||||||
|
]);
|
||||||
// Auto-blacklist if too many attempts
|
} catch (PDOException $e) {
|
||||||
if (!$this->isAllowed($username, $ipAddress)) {
|
|
||||||
$this->addToBlacklist(
|
|
||||||
$ipAddress,
|
|
||||||
false,
|
|
||||||
'Auto-blacklisted due to excessive login attempts',
|
|
||||||
'system',
|
|
||||||
null,
|
|
||||||
$this->autoBlacklistDuration
|
|
||||||
);
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -448,6 +454,13 @@ class RateLimiter {
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$result = $stmt->fetch(PDO::FETCH_ASSOC);
|
$result = $stmt->fetch(PDO::FETCH_ASSOC);
|
||||||
|
|
||||||
|
// Also check what's in the table
|
||||||
|
$sql = "SELECT * FROM {$this->authRatelimitTable} WHERE ip_address = :ip";
|
||||||
|
$stmt = $this->db->prepare($sql);
|
||||||
|
$stmt->execute([':ip' => $ipAddress]);
|
||||||
|
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC);
|
||||||
|
|
||||||
return $result['attempts'] >= $this->maxAttempts;
|
return $result['attempts'] >= $this->maxAttempts;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -9,12 +9,13 @@ require_once __DIR__ . '/../helpers/logs.php';
|
||||||
* @param Database $database Database connection
|
* @param Database $database Database connection
|
||||||
* @param string $endpoint The endpoint being accessed
|
* @param string $endpoint The endpoint being accessed
|
||||||
* @param int|null $userId Current user ID if authenticated
|
* @param int|null $userId Current user ID if authenticated
|
||||||
|
* @param RateLimiter|null $existingRateLimiter Optional existing RateLimiter instance
|
||||||
* @return bool True if request is allowed, false if rate limited
|
* @return bool True if request is allowed, false if rate limited
|
||||||
*/
|
*/
|
||||||
function checkRateLimit($database, $endpoint, $userId = null) {
|
function checkRateLimit($database, $endpoint, $userId = null, $existingRateLimiter = null) {
|
||||||
global $app_root;
|
global $app_root;
|
||||||
$isTest = defined('PHPUNIT_RUNNING');
|
$isTest = defined('PHPUNIT_RUNNING');
|
||||||
$rateLimiter = new RateLimiter($database);
|
$rateLimiter = $existingRateLimiter ?? new RateLimiter($database);
|
||||||
$ipAddress = getUserIP();
|
$ipAddress = getUserIP();
|
||||||
|
|
||||||
// Check if request is allowed
|
// Check if request is allowed
|
||||||
|
|
|
@ -25,12 +25,11 @@ try {
|
||||||
require_once '../app/classes/ratelimiter.php';
|
require_once '../app/classes/ratelimiter.php';
|
||||||
$rateLimiter = new RateLimiter($dbWeb);
|
$rateLimiter = new RateLimiter($dbWeb);
|
||||||
|
|
||||||
|
// Get user IP
|
||||||
|
$user_IP = getUserIP();
|
||||||
|
|
||||||
if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) {
|
if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) {
|
||||||
try {
|
try {
|
||||||
// apply page rate limiting
|
|
||||||
require_once '../app/includes/rate_limit_middleware.php';
|
|
||||||
checkRateLimit($dbWeb, 'login', null); // null since user is not logged in yet
|
|
||||||
|
|
||||||
// Validate form data
|
// Validate form data
|
||||||
$security = SecurityHelper::getInstance();
|
$security = SecurityHelper::getInstance();
|
||||||
$formData = $security->sanitizeArray($_POST, ['username', 'password', 'remember_me', 'csrf_token']);
|
$formData = $security->sanitizeArray($_POST, ['username', 'password', 'remember_me', 'csrf_token']);
|
||||||
|
@ -57,17 +56,20 @@ try {
|
||||||
$username = $formData['username'];
|
$username = $formData['username'];
|
||||||
$password = $formData['password'];
|
$password = $formData['password'];
|
||||||
|
|
||||||
// Check if IP is blacklisted
|
// Skip all checks if IP is whitelisted
|
||||||
if ($rateLimiter->isIpBlacklisted($user_IP)) {
|
|
||||||
throw new Exception(Feedback::get('LOGIN', 'IP_BLACKLISTED')['message']);
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check rate limiting (but skip if IP is whitelisted)
|
|
||||||
if (!$rateLimiter->isIpWhitelisted($user_IP)) {
|
if (!$rateLimiter->isIpWhitelisted($user_IP)) {
|
||||||
$attempts = $rateLimiter->getRecentAttempts($user_IP);
|
// Check if IP is blacklisted
|
||||||
if ($attempts >= $rateLimiter->maxAttempts) {
|
if ($rateLimiter->isIpBlacklisted($user_IP)) {
|
||||||
|
throw new Exception(Feedback::get('LOGIN', 'IP_BLACKLISTED')['message']);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check rate limiting before recording attempt
|
||||||
|
if ($rateLimiter->tooManyAttempts($username, $user_IP)) {
|
||||||
throw new Exception(Feedback::get('LOGIN', 'LOGIN_BLOCKED')['message']);
|
throw new Exception(Feedback::get('LOGIN', 'LOGIN_BLOCKED')['message']);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Record this attempt
|
||||||
|
$rateLimiter->attempt($username, $user_IP);
|
||||||
}
|
}
|
||||||
|
|
||||||
// login successful
|
// login successful
|
||||||
|
@ -77,22 +79,12 @@ try {
|
||||||
// 30*24*60*60 = 30 days
|
// 30*24*60*60 = 30 days
|
||||||
$cookie_lifetime = 30 * 24 * 60 * 60;
|
$cookie_lifetime = 30 * 24 * 60 * 60;
|
||||||
$setcookie_lifetime = time() + 30 * 24 * 60 * 60;
|
$setcookie_lifetime = time() + 30 * 24 * 60 * 60;
|
||||||
$gc_maxlifetime = 30 * 24 * 60 * 60;
|
|
||||||
} else {
|
} else {
|
||||||
// 0 - session end on browser close
|
// 0 - session end on browser close
|
||||||
// 1440 - 24 minutes (default)
|
|
||||||
$cookie_lifetime = 0;
|
$cookie_lifetime = 0;
|
||||||
$setcookie_lifetime = 0;
|
$setcookie_lifetime = 0;
|
||||||
$gc_maxlifetime = 1440;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Configure secure session settings
|
|
||||||
ini_set('session.cookie_httponly', 1);
|
|
||||||
ini_set('session.use_only_cookies', 1);
|
|
||||||
ini_set('session.cookie_secure', isset($_SERVER['HTTPS']) ? 1 : 0);
|
|
||||||
ini_set('session.cookie_samesite', 'Strict');
|
|
||||||
ini_set('session.gc_maxlifetime', $gc_maxlifetime);
|
|
||||||
|
|
||||||
// Regenerate session ID to prevent session fixation
|
// Regenerate session ID to prevent session fixation
|
||||||
session_regenerate_id(true);
|
session_regenerate_id(true);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue