From: Dan Untenzu <untenzu@webit.de>
Date: Fri, 17 Mar 2017 16:28:56 +0000 (+0100)
Subject: [BUGFIX] Remove false IP challenge to allow uploads again
X-Git-Tag: 3.1.0~1
X-Git-Url: https://git.p6c8.net/jirafeau_mojo42.git/commitdiff_plain/b402561271f0de4f4c12fc8f8c7d51d12e0a3e5c

[BUGFIX] Remove false IP challenge to allow uploads again

The upload form currently has a broken IP check
which dissallows all uploads even if no IP restrictions
are configured in the config.

Replace the wrong check and add some code comments
to make this authorization more readable.

Refs #113 #107 #111
---

diff --git a/index.php b/index.php
index 7c24e3b..09b37e4 100644
--- a/index.php
+++ b/index.php
@@ -34,33 +34,31 @@ if (has_error()) {
 require(JIRAFEAU_ROOT . 'lib/template/header.php');
 
 /* Check if user is allowed to upload. */
-if (!jirafeau_challenge_upload_ip($cfg, get_ip_address($cfg)) ||
-    count($cfg['upload_ip']) == 0) {
-
-    /* Ask password if upload password is set. */
+// First check: Challenge by IP
+if (true === jirafeau_challenge_upload_ip($cfg['upload_ip'], get_ip_address($cfg))) {
+    // Is an upload password required?
     if (jirafeau_has_upload_password($cfg)) {
         session_start();
 
-        /* Unlog if asked. */
+        // Logout action
         if (isset($_POST['action']) && (strcmp($_POST['action'], 'logout') == 0)) {
             session_unset();
         }
 
-        /* Auth. */
+        // Challenge by password
+        // …save successful logins in session
         if (isset($_POST['upload_password'])) {
             if (jirafeau_challenge_upload_password($cfg, $_POST['upload_password'])) {
                 $_SESSION['upload_auth'] = true;
                 $_SESSION['user_upload_password'] = $_POST['upload_password'];
             } else {
                 $_SESSION['admin_auth'] = false;
-                echo '<div class="error"><p>' . t('Wrong password.') . '</p></div>';
-                require(JIRAFEAU_ROOT.'lib/template/footer.php');
-                exit;
+                jirafeau_fatal_error(t('Wrong password.'), $cfg);
             }
         }
 
-        /* Show auth page. */
-        if (!isset($_SESSION['upload_auth']) || $_SESSION['upload_auth'] != true) {
+        // Show login form if user session is not authorized yet
+        if (true === empty($_SESSION['upload_auth'])) {
             ?>
             <form method="post">
             <fieldset>
@@ -88,12 +86,11 @@ if (!jirafeau_challenge_upload_ip($cfg, get_ip_address($cfg)) ||
             require(JIRAFEAU_ROOT.'lib/template/footer.php');
             exit;
         }
-    } else {
-            echo '<div class="error"><p>' . t('Access denied') . '</p></div>';
-            require(JIRAFEAU_ROOT.'lib/template/footer.php');
-            exit;
     }
-} 
+}
+else {
+    jirafeau_fatal_error(t('Access denied'), $cfg);
+}
 
 ?>
 <div id="upload_finished">
diff --git a/lib/functions.php b/lib/functions.php
index d844e50..7d15e1b 100644
--- a/lib/functions.php
+++ b/lib/functions.php
@@ -1070,22 +1070,26 @@ function jirafeau_challenge_upload_password($cfg, $password)
 
 /**
  * Test if visitor's IP is authorized to upload.
- * @param $ip IP to be challenged
+ *
+ * @param $allowedIpList array of allowed IPs
+ * @param $challengedIp IP to be challenged
  * @return true if IP is authorized, false otherwise.
  */
-function jirafeau_challenge_upload_ip($cfg, $ip)
+function jirafeau_challenge_upload_ip($allowedIpList, $challengedIp)
 {
-    if (count($cfg['upload_ip']) == 0) {
+    // skip if list is empty = all IPs allowed
+    if (count($allowedIpList) == 0) {
         return true;
     }
-    foreach ($cfg['upload_ip'] as $i) {
-        if ($i == $ip) {
+    // test given IP against each allowed IP
+    foreach ($allowedIpList as $i) {
+        if ($i == $challengedIp) {
             return true;
         }
         // CIDR test for IPv4 only.
         if (strpos($i, '/') !== false) {
             list($subnet, $mask) = explode('/', $i);
-            if ((ip2long($ip) & ~((1 << (32 - $mask)) - 1)) == ip2long($subnet)) {
+            if ((ip2long($challengedIp) & ~((1 << (32 - $mask)) - 1)) == ip2long($subnet)) {
                 return true;
             }
         }