Fixes errors in ratelimiter

main
Yasen Pramatarov 2025-02-21 11:44:52 +02:00
parent 4182ba6c1b
commit 487c23da3e
6 changed files with 42 additions and 33 deletions

View File

@ -123,7 +123,20 @@ class RateLimiter {
*/ */
public function isIpBlacklisted($ip) { public function isIpBlacklisted($ip) {
// First check if IP is explicitly blacklisted or in a blacklisted range // First check if IP is explicitly blacklisted or in a blacklisted range
$stmt = $this->db->prepare("SELECT ip_address, is_network, expiry_time FROM {$this->blacklistTable}"); $stmt = $this->db->prepare("SELECT ip_address, is_network, expiry_time FROM {$this->blacklistTable} WHERE ip_address = ?");
$stmt->execute([$ip]);
$row = $stmt->fetch(PDO::FETCH_ASSOC);
if ($row) {
// Skip expired entries
if ($row['expiry_time'] !== null && strtotime($row['expiry_time']) < time()) {
return false;
}
return true;
}
// Check network ranges
$stmt = $this->db->prepare("SELECT ip_address, expiry_time FROM {$this->blacklistTable} WHERE is_network = 1");
$stmt->execute(); $stmt->execute();
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) { while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
@ -132,15 +145,9 @@ class RateLimiter {
continue; continue;
} }
if ($row['is_network']) {
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;
}
}
} }
return false; return false;
@ -307,6 +314,7 @@ class RateLimiter {
// Remove the IP // Remove the IP
$stmt = $this->db->prepare("DELETE FROM {$this->blacklistTable} WHERE ip_address = ?"); $stmt = $this->db->prepare("DELETE FROM {$this->blacklistTable} WHERE ip_address = ?");
$result = $stmt->execute([$ip]); $result = $stmt->execute([$ip]);
if ($result && $ipDetails) { if ($result && $ipDetails) {
@ -495,12 +503,12 @@ class RateLimiter {
// Get limit based on endpoint type and user role // Get limit based on endpoint type and user role
$limit = $this->getPageLimitForEndpoint($endpoint, $userId); $limit = $this->getPageLimitForEndpoint($endpoint, $userId);
// Count recent requests // Count recent requests, including this one
$sql = "SELECT COUNT(*) as request_count $sql = "SELECT COUNT(*) as request_count
FROM {$this->pagesRatelimitTable} FROM {$this->pagesRatelimitTable}
WHERE ip_address = :ip WHERE ip_address = :ip
AND endpoint = :endpoint AND endpoint = :endpoint
AND request_time > DATETIME('now', '-1 minute')"; AND request_time >= DATETIME('now', '-1 minute')";
$stmt = $this->db->prepare($sql); $stmt = $this->db->prepare($sql);
$stmt->execute([ $stmt->execute([

View File

@ -12,7 +12,7 @@ require_once __DIR__ . '/../classes/ratelimiter.php';
*/ */
function checkRateLimit($database, $endpoint, $userId = null) { function checkRateLimit($database, $endpoint, $userId = null) {
$isTest = defined('PHPUNIT_RUNNING'); $isTest = defined('PHPUNIT_RUNNING');
$rateLimiter = new RateLimiter($database['db']); $rateLimiter = new RateLimiter($database);
$ipAddress = $_SERVER['REMOTE_ADDR']; $ipAddress = $_SERVER['REMOTE_ADDR'];
// Check if request is allowed // Check if request is allowed

View File

@ -19,11 +19,11 @@ unset($error);
try { try {
// connect to database // connect to database
$dbWeb = connectDB($config); $dbWeb = connectDB($config)['db'];
// Initialize RateLimiter // Initialize RateLimiter
require_once '../app/classes/ratelimiter.php'; require_once '../app/classes/ratelimiter.php';
$rateLimiter = new RateLimiter($dbWeb['db']); $rateLimiter = new RateLimiter($dbWeb);
if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) { if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) {
try { try {
@ -54,8 +54,8 @@ try {
throw new Exception("Invalid input: " . implode(", ", $errors)); throw new Exception("Invalid input: " . implode(", ", $errors));
} }
$username = $_POST['username']; $username = $formData['username'];
$password = $_POST['password']; $password = $formData['password'];
// Check if IP is blacklisted // Check if IP is blacklisted
if ($rateLimiter->isIpBlacklisted($user_IP)) { if ($rateLimiter->isIpBlacklisted($user_IP)) {
@ -73,7 +73,7 @@ try {
// login successful // login successful
if ( $userObject->login($username, $password) ) { if ( $userObject->login($username, $password) ) {
// if remember_me is checked, max out the session // if remember_me is checked, max out the session
if (isset($_POST['remember_me'])) { if (isset($formData['remember_me'])) {
// 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;
@ -119,7 +119,7 @@ try {
$logObject->insertLog($user_id, "Login: User \"$username\" logged in. IP: $user_IP", 'user'); $logObject->insertLog($user_id, "Login: User \"$username\" logged in. IP: $user_IP", 'user');
// Set success message and redirect // Set success message and redirect
Feedback::flash('LOGIN', 'LOGIN_SUCCESS', null, true); Feedback::flash('LOGIN', 'LOGIN_SUCCESS');
header('Location: ' . htmlspecialchars($app_root)); header('Location: ' . htmlspecialchars($app_root));
exit(); exit();
} else { } else {
@ -140,7 +140,7 @@ try {
// Show configured login message if any // Show configured login message if any
if (!empty($config['login_message'])) { if (!empty($config['login_message'])) {
echo Feedback::render('NOTICE', 'DEFAULT', $config['login_message'], false, false, false); echo Feedback::render('NOTICE', 'DEFAULT', $config['login_message'], false);
} }
// Get any new feedback messages // Get any new feedback messages

View File

@ -14,7 +14,7 @@ if ($config['registration_enabled'] == true) {
try { try {
// connect to database // connect to database
$dbWeb = connectDB($config); $dbWeb = connectDB($config)['db'];
if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) { if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) {

View File

@ -30,7 +30,7 @@ require_once '../app/helpers/security.php';
$security = SecurityHelper::getInstance(); $security = SecurityHelper::getInstance();
// Verify CSRF token for POST requests // Verify CSRF token for POST requests
verifyCsrfToken(); applyCsrfMiddleware();
// Initialize feedback message system // Initialize feedback message system
require_once '../app/classes/feedback.php'; require_once '../app/classes/feedback.php';

View File

@ -2,6 +2,7 @@
require_once dirname(__DIR__, 3) . '/app/classes/database.php'; require_once dirname(__DIR__, 3) . '/app/classes/database.php';
require_once dirname(__DIR__, 3) . '/app/classes/ratelimiter.php'; require_once dirname(__DIR__, 3) . '/app/classes/ratelimiter.php';
require_once dirname(__DIR__, 3) . '/app/classes/log.php';
require_once dirname(__DIR__, 3) . '/app/includes/rate_limit_middleware.php'; require_once dirname(__DIR__, 3) . '/app/includes/rate_limit_middleware.php';
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
@ -75,7 +76,7 @@ class RateLimitMiddlewareTest extends TestCase
{ {
// Test multiple requests // Test multiple requests
for ($i = 1; $i <= 5; $i++) { for ($i = 1; $i <= 5; $i++) {
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
if ($i <= 5) { if ($i <= 5) {
// First 5 requests should pass // First 5 requests should pass
@ -91,7 +92,7 @@ class RateLimitMiddlewareTest extends TestCase
{ {
// Test AJAX request bypass // Test AJAX request bypass
$_SERVER['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'; $_SERVER['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest';
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
$this->assertTrue($result); $this->assertTrue($result);
} }
@ -99,14 +100,14 @@ class RateLimitMiddlewareTest extends TestCase
{ {
// Use up the rate limit // Use up the rate limit
for ($i = 0; $i < 5; $i++) { for ($i = 0; $i < 5; $i++) {
checkRateLimit(['db' => $this->db], '/login'); checkRateLimit($this->db, '/login');
} }
// Wait for rate limit to reset (use a short window for testing) // Wait for rate limit to reset (use a short window for testing)
sleep(2); sleep(2);
// Should be able to make request again // Should be able to make request again
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
$this->assertTrue($result); $this->assertTrue($result);
} }
@ -114,11 +115,11 @@ class RateLimitMiddlewareTest extends TestCase
{ {
// Use up rate limit for login // Use up rate limit for login
for ($i = 0; $i < 5; $i++) { for ($i = 0; $i < 5; $i++) {
checkRateLimit(['db' => $this->db], '/login'); checkRateLimit($this->db, '/login');
} }
// Should still be able to access different endpoint // Should still be able to access different endpoint
$result = checkRateLimit(['db' => $this->db], '/dashboard'); $result = checkRateLimit($this->db, '/dashboard');
$this->assertTrue($result); $this->assertTrue($result);
} }
@ -127,12 +128,12 @@ class RateLimitMiddlewareTest extends TestCase
// Use up rate limit for first IP // Use up rate limit for first IP
$_SERVER['REMOTE_ADDR'] = '127.0.0.1'; $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
for ($i = 0; $i < 5; $i++) { for ($i = 0; $i < 5; $i++) {
checkRateLimit(['db' => $this->db], '/login'); checkRateLimit($this->db, '/login');
} }
// Different IP should not be affected // Different IP should not be affected
$_SERVER['REMOTE_ADDR'] = '127.0.0.2'; $_SERVER['REMOTE_ADDR'] = '127.0.0.2';
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
$this->assertTrue($result); $this->assertTrue($result);
} }
@ -146,7 +147,7 @@ class RateLimitMiddlewareTest extends TestCase
// Should be able to make more requests than limit // Should be able to make more requests than limit
for ($i = 0; $i < 10; $i++) { for ($i = 0; $i < 10; $i++) {
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
$this->assertTrue($result); $this->assertTrue($result);
} }
} }
@ -160,7 +161,7 @@ class RateLimitMiddlewareTest extends TestCase
); );
// Should be blocked immediately // Should be blocked immediately
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
$this->assertFalse($result); $this->assertFalse($result);
} }
@ -168,7 +169,7 @@ class RateLimitMiddlewareTest extends TestCase
{ {
// Use up some of the rate limit // Use up some of the rate limit
for ($i = 0; $i < 2; $i++) { for ($i = 0; $i < 2; $i++) {
checkRateLimit(['db' => $this->db], '/login'); checkRateLimit($this->db, '/login');
} }
// Destroy and restart session // Destroy and restart session
@ -176,7 +177,7 @@ class RateLimitMiddlewareTest extends TestCase
//session_start(); //session_start();
// Should still count previous requests // Should still count previous requests
$result = checkRateLimit(['db' => $this->db], '/login'); $result = checkRateLimit($this->db, '/login');
$this->assertTrue($result); $this->assertTrue($result);
} }
} }