From 65750990b827cdc907e408d62f39026428c81858 Mon Sep 17 00:00:00 2001
From: Carsten Schmitz <carsten.schmitz@limesurvey.org>
Date: Thu, 25 May 2023 23:17:38 +0200
Subject: [PATCH] Fixed issue: [security] Unsafe way to detect IP address
 against brute-force attacks

---
 application/extensions/LimeDebug/LimeDebug.php |  2 +-
 application/helpers/common_helper.php          | 16 ++++++++++++++++
 application/models/FailedLoginAttempt.php      |  6 +++---
 tests/unit/models/FailedLoginAttemptTest.php   |  2 +-
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/application/extensions/LimeDebug/LimeDebug.php b/application/extensions/LimeDebug/LimeDebug.php
index 6df3dc29271..c7b8db84ebf 100644
--- a/application/extensions/LimeDebug/LimeDebug.php
+++ b/application/extensions/LimeDebug/LimeDebug.php
@@ -7,7 +7,7 @@ class LimeDebug extends CWidget
     {
         public function run()
         {
-            if (YII_DEBUG && in_array(getIPAddress(), array("127.0.0.1","::1")))
+            if (YII_DEBUG && in_array(getRealIPAddress(), array("127.0.0.1","::1")))
             {                
                 $data = array(
                     'session' => $_SESSION,
diff --git a/application/helpers/common_helper.php b/application/helpers/common_helper.php
index 71b38ec5731..094e0dd672b 100644
--- a/application/helpers/common_helper.php
+++ b/application/helpers/common_helper.php
@@ -4610,6 +4610,22 @@ function getIPAddress()
     return $sIPAddress;
 }
 
+
+/**
+ * This function returns the real IP address and should mainly be used for security sensitive purposes
+ * If you want to use the IP address for language detection or similar, use getIPAddress() instead
+ * 
+ * @return  string  Client IP Address
+ */
+function getRealIPAddress()
+{
+    $sIPAddress = '127.0.0.1';
+    if (!empty($_SERVER['REMOTE_ADDR']) && filter_var($_SERVER['REMOTE_ADDR'], FILTER_VALIDATE_IP) !== false) {
+        $sIPAddress = $_SERVER['REMOTE_ADDR'];
+    }
+    return $sIPAddress;
+}
+
 /**
 * This function tries to find out a valid language code for the language of the browser used
 * If it cannot find it it will return the default language from global settings
diff --git a/application/models/FailedLoginAttempt.php b/application/models/FailedLoginAttempt.php
index 923f7edb3e6..e9350f252a6 100644
--- a/application/models/FailedLoginAttempt.php
+++ b/application/models/FailedLoginAttempt.php
@@ -63,7 +63,7 @@ public function tableName()
      */
     public function deleteAttempts(string $attemptType = 'login')
     {
-        $ip = substr(getIPAddress(), 0, 40);
+        $ip = substr(getRealIPAddress(), 0, 40);
 
         if (Yii::app()->getConfig('DBVersion') <= 480) {
             $this->deleteAllByAttributes(array('ip' => $ip));
@@ -82,7 +82,7 @@ public function deleteAttempts(string $attemptType = 'login')
     public function isLockedOut(string $attemptType): bool
     {
         $isLockedOut = false;
-        $ip = substr(getIPAddress(), 0, 40);
+        $ip = substr(getRealIPAddress(), 0, 40);
 
         // Return false if IP is whitelisted
         if ($this->isWhitelisted($ip, $attemptType)) {
@@ -144,7 +144,7 @@ public function addAttempt($attemptType = self::TYPE_LOGIN)
     {
         if (!$this->isLockedOut($attemptType)) {
             $timestamp = date("Y-m-d H:i:s");
-            $ip = substr(getIPAddress(), 0, 40);
+            $ip = substr(getRealIPAddress(), 0, 40);
 
             if (Yii::app()->getConfig('DBVersion') <= 480) {
                 $row = $this->findByAttributes(array('ip' => $ip));
diff --git a/tests/unit/models/FailedLoginAttemptTest.php b/tests/unit/models/FailedLoginAttemptTest.php
index c48297d078a..d0f7d534280 100644
--- a/tests/unit/models/FailedLoginAttemptTest.php
+++ b/tests/unit/models/FailedLoginAttemptTest.php
@@ -9,7 +9,7 @@ class FailedLoginAttempTest extends TestBaseClass
     public function testAddDeleteAttemp()
     {
         // Save Ip
-        $ip = substr(getIPAddress(), 0, 40);
+        $ip = substr(getRealIPAddress(), 0, 40);
 
         // Make sure there are no records for the ip
         FailedLoginAttempt::model()->deleteAttempts(FailedLoginAttempt::TYPE_LOGIN);