From b32f397949cbca67f22f90ef940e0d658ba057ab Mon Sep 17 00:00:00 2001
From: Jakub Onderka <jakub.onderka@gmail.com>
Date: Wed, 23 Feb 2022 09:52:59 +0100
Subject: [PATCH] fix: [internal] CIDR validation

---
 app/Lib/Tools/CidrTool.php | 12 +++++++++---
 app/Model/AuthKey.php      | 22 ++++++++++++----------
 app/Test/CidrToolTest.php  | 11 +++++++++++
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/app/Lib/Tools/CidrTool.php b/app/Lib/Tools/CidrTool.php
index b21351c0c..36d8b83c8 100644
--- a/app/Lib/Tools/CidrTool.php
+++ b/app/Lib/Tools/CidrTool.php
@@ -78,9 +78,15 @@ class CidrTool
             return false;
         }
 
-        $maximumNetmask = strlen($ipBytes) === 4 ? 32 : 128;
-        if (isset($parts[1]) && ($parts[1] > $maximumNetmask || $parts[1] < 0)) {
-            return false; // Netmask part of CIDR is invalid
+        if (isset($parts[1])) {
+            if (!ctype_digit($parts[1])) {
+                return false;
+            }
+
+            $maximumNetmask = strlen($ipBytes) === 4 ? 32 : 128;
+            if ($parts[1] > $maximumNetmask || $parts[1] < 0) {
+                return false; // Netmask part of CIDR is invalid
+            }
         }
 
         return true;
diff --git a/app/Model/AuthKey.php b/app/Model/AuthKey.php
index 63990e97f..b06e902f9 100644
--- a/app/Model/AuthKey.php
+++ b/app/Model/AuthKey.php
@@ -2,6 +2,7 @@
 App::uses('AppModel', 'Model');
 App::uses('RandomTool', 'Tools');
 App::uses('CidrTool', 'Tools');
+App::uses('JsonTool', 'Tools');
 App::uses('BlowfishConstantPasswordHasher', 'Controller/Component/Auth');
 
 /**
@@ -47,19 +48,20 @@ class AuthKey extends AppModel
         }
 
         if (!empty($this->data['AuthKey']['allowed_ips'])) {
-            if (is_string($this->data['AuthKey']['allowed_ips'])) {
-                $this->data['AuthKey']['allowed_ips'] = trim($this->data['AuthKey']['allowed_ips']);
-                if (empty($this->data['AuthKey']['allowed_ips'])) {
-                    $this->data['AuthKey']['allowed_ips'] = [];
+            $allowedIps = &$this->data['AuthKey']['allowed_ips'];
+            if (is_string($allowedIps)) {
+                $allowedIps = trim($allowedIps);
+                if (empty($allowedIps)) {
+                    $allowedIps = [];
                 } else {
-                    $this->data['AuthKey']['allowed_ips'] = explode("\n", $this->data['AuthKey']['allowed_ips']);
-                    $this->data['AuthKey']['allowed_ips'] = array_map('trim', $this->data['AuthKey']['allowed_ips']);
+                    $allowedIps = preg_split('/([\n,])/', $allowedIps);
+                    $allowedIps = array_map('trim', $allowedIps);
                 }
             }
-            if (!is_array($this->data['AuthKey']['allowed_ips'])) {
+            if (!is_array($allowedIps)) {
                 $this->invalidate('allowed_ips', 'Allowed IPs must be array');
             }
-            foreach ($this->data['AuthKey']['allowed_ips'] as $cidr) {
+            foreach ($allowedIps as $cidr) {
                 if (!CidrTool::validate($cidr)) {
                     $this->invalidate('allowed_ips', "$cidr is not valid IP range");
                 }
@@ -91,7 +93,7 @@ class AuthKey extends AppModel
     {
         foreach ($results as $key => $val) {
             if (isset($val['AuthKey']['allowed_ips'])) {
-                $results[$key]['AuthKey']['allowed_ips'] = $this->jsonDecode($val['AuthKey']['allowed_ips']);
+                $results[$key]['AuthKey']['allowed_ips'] = JsonTool::decode($val['AuthKey']['allowed_ips']);
             }
         }
         return $results;
@@ -103,7 +105,7 @@ class AuthKey extends AppModel
             if (empty($this->data['AuthKey']['allowed_ips'])) {
                 $this->data['AuthKey']['allowed_ips'] = null;
             } else {
-                $this->data['AuthKey']['allowed_ips'] = json_encode($this->data['AuthKey']['allowed_ips']);
+                $this->data['AuthKey']['allowed_ips'] = JsonTool::encode($this->data['AuthKey']['allowed_ips']);
             }
         }
         return true;
diff --git a/app/Test/CidrToolTest.php b/app/Test/CidrToolTest.php
index 2ea58bc29..d825962a4 100644
--- a/app/Test/CidrToolTest.php
+++ b/app/Test/CidrToolTest.php
@@ -5,6 +5,17 @@ use PHPUnit\Framework\TestCase;
 
 class CidrToolTest extends TestCase
 {
+    public function testValidate(): void
+    {
+        $this->assertTrue(CidrTool::validate('1.2.3.4'));
+        $this->assertTrue(CidrTool::validate('1.2.3.4/32'));
+        $this->assertTrue(CidrTool::validate('::1'));
+        $this->assertTrue(CidrTool::validate('::1/128'));
+        $this->assertFalse(CidrTool::validate('::1/a'));
+        $this->assertFalse(CidrTool::validate('1.2.3.4/a'));
+        $this->assertFalse(CidrTool::validate('1.2.3.4/32, 1.2.3.4'));
+    }
+
     public function testEmptyList(): void
     {
         $cidrTool = new CidrTool([]);