From: Erik Lundin <erik@lun.nu>
Date: Tue, 12 Nov 2019 00:04:45 +0000 (+0100)
Subject: Fix upload IP check
X-Git-Tag: 4.1.0~7
X-Git-Url: https://git.p6c8.net/jirafeau_mojo42.git/commitdiff_plain/a00401257a73daf0449386e57ee9a811e2a4f408?ds=inline

Fix upload IP check

If no IP addresses are listed at 'upload_ip_nopassword', index.php won't
show any login form and indicate the session as authenticated. During
the actual upload, script.php will show the error 'Error 2: No password
nor allowed IP'.

The reason is that jirafeau_challenge_upload_ip returns true if the
supplied IP whitelist is empty, but uploading without a password doesn't
work this way (and shouldn't).

I took the liberty to split 'jirafeau_challenge_upload_ip' into one
function for the normal IP check, and one for checking if allowed to
upload without a password. Having function names that clearly
communicates the intent makes it easier to avoid bugs.
---

diff --git a/index.php b/index.php
index b03af76..832749b 100644
--- a/index.php
+++ b/index.php
@@ -36,13 +36,13 @@ require(JIRAFEAU_ROOT . 'lib/template/header.php');
 
 /* Check if user is allowed to upload. */
 // First check: Challenge by IP NO PASSWORD
-if (true === jirafeau_challenge_upload_ip($cfg['upload_ip_nopassword'], get_ip_address($cfg))) {
+if (true === jirafeau_challenge_upload_ip_without_password($cfg, get_ip_address($cfg))) {
     $_SESSION['upload_auth'] = true;
     $_POST['upload_password'] = '';
     $_SESSION['user_upload_password'] = $_POST['upload_password'];
 }
 // Second check: Challenge by IP
-elseif (true === jirafeau_challenge_upload_ip($cfg['upload_ip'], get_ip_address($cfg))) {
+elseif (true === jirafeau_challenge_upload_ip($cfg, get_ip_address($cfg))) {
     // Is an upload password required?
     if (jirafeau_has_upload_password($cfg)) {
         // Logout action
@@ -274,7 +274,7 @@ else {
 
     <?php
     if (jirafeau_has_upload_password($cfg)
-        && false === jirafeau_challenge_upload_ip($cfg['upload_ip_nopassword'], get_ip_address($cfg))) {
+        && false === jirafeau_challenge_upload_ip_without_password($cfg, get_ip_address($cfg))) {
         ?>
     <form method="post" class="form logout">
         <input type = "hidden" name = "action" value = "logout"/>
diff --git a/lib/config.original.php b/lib/config.original.php
index 30440a2..913f3ac 100644
--- a/lib/config.original.php
+++ b/lib/config.original.php
@@ -85,7 +85,6 @@ $cfg['upload_password'] = array();
 $cfg['upload_ip'] = array();
 
 /* List of IP allowed to upload a file without password.
- * If the list is empty, then there is no upload restriction based on IP.
  * Elements of the list can be a single IP (e.g. "123.45.67.89") or
  * an IP range (e.g. "123.45.0.0/16").
  * Note that CIDR notation is available for IPv4 only for the moment.
diff --git a/lib/functions.php b/lib/functions.php
index c234f4c..bff319f 100644
--- a/lib/functions.php
+++ b/lib/functions.php
@@ -1074,19 +1074,14 @@ function jirafeau_challenge_upload_password($cfg, $password)
 }
 
 /**
- * Test if visitor's IP is authorized to upload.
+ * Test if the given IP is whitelisted by the given list.
  *
  * @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($allowedIpList, $challengedIp)
+function jirafeau_challenge_ip($allowedIpList, $challengedIp)
 {
-    // skip if list is empty = all IPs allowed
-    if (count($allowedIpList) == 0) {
-        return true;
-    }
-    // test given IP against each allowed IP
     foreach ($allowedIpList as $i) {
         if ($i == $challengedIp) {
             return true;
@@ -1102,6 +1097,42 @@ function jirafeau_challenge_upload_ip($allowedIpList, $challengedIp)
     return false;
 }
 
+/**
+ * Check if Jirafeau has a restriction on the IP address for uploading.
+ * @return true if uploading is IP restricted, false otherwise.
+ */
+function jirafeau_upload_has_ip_restriction($cfg) {
+    return count($cfg['upload_ip']) > 0;
+}
+
+/**
+ * Test if visitor's IP is authorized to upload at all.
+ *
+ * @param $cfg configuration
+ * @param $challengedIp IP to be challenged
+ * @return true if IP is authorized, false otherwise.
+ */
+function jirafeau_challenge_upload_ip($cfg, $challengedIp)
+{
+    // If no IP address have been listed, allow upload from any IP
+    if (!jirafeau_upload_has_ip_restriction($cfg)) {
+        return true;
+    }
+    return jirafeau_challenge_ip($cfg['upload_ip'], $challengedIp);
+}
+
+/**
+ * Test if visitor's IP is authorized to upload without a password.
+ *
+ * @param $cfg configuration
+ * @param $challengedIp IP to be challenged
+ * @return true if IP is authorized, false otherwise.
+ */
+function jirafeau_challenge_upload_ip_without_password($cfg, $challengedIp)
+{
+    return jirafeau_challenge_ip($cfg['upload_ip_nopassword'], $challengedIp);
+}
+
 /**
  * Test if visitor's IP is authorized or password is supplied and authorized
  * @param $ip IP to be challenged