Add checks

Fix #116

Author: tsmr

hook.php

@@ -476,7 +476,8 @@ function plugin_addressing_addOrderBy($itemtype, $ID, $order, $key)
 {
     if ($itemtype == Addressing::class
         && ($ID == 100 || $ID == 101)) {
-        return "ORDER BY INET_ATON(ITEM_$key) $order";
+        // $key holds the namespaced itemtype: quote the alias as the SELECT does, or the backslashes break the query
+        return "ORDER BY INET_ATON(`ITEM_$key`) $order";
     }
 }
 

phpstan-baseline.neon

@@ -36,12 +36,6 @@ parameters:
 			count: 1
 			path: src/Addressing.php
 
-		-
-			message: '#^Comparison operation "\>" between 0 and 0 is always false\.$#'
-			identifier: greater.alwaysFalse
-			count: 1
-			path: src/Addressing.php
-
 		-
 			message: '#^Instantiating an object from an unrestricted dynamic string is forbidden \(see https\://github\.com/glpi\-project/phpstan\-glpi\?tab\=readme\-ov\-file\#forbiddynamicinstantiationrule\)\.$#'
 			identifier: glpi.forbidDynamicInstantiation
@@ -72,12 +66,6 @@ parameters:
 			count: 1
 			path: src/Addressing.php
 
-		-
-			message: '#^Result of && is always false\.$#'
-			identifier: booleanAnd.alwaysFalse
-			count: 3
-			path: src/Addressing.php
-
 		-
 			message: '#^Unreachable statement \- code above always terminates\.$#'
 			identifier: deadCode.unreachable

src/Addressing.php

@@ -975,26 +975,16 @@ public function showReport($params)
     {
         $Report = new Report();
 
-        // Default values of parameters
-        $default_values["start"] = $start = 0;
-        $default_values["id"] = $id = 0;
-        $default_values["export"] = $export = false;
-        $default_values['filter'] = $filter = 0;
-
-        foreach ($default_values as $key => $val) {
-            if (isset($params[$key])) {
-                $$key = $params[$key];
-            }
-        }
-
-        // getFromDB() does not cast its argument, and MySQL coerces a string to a number
-        // when comparing it to an INT column: "1-alert(1)" would match the row with id 1
-        // and then reach the template. Force the integer types here, before any use.
-        // $start is also used in pagination arithmetic, which raises a TypeError in PHP 8
-        // on a non numeric value and lets a negative offset walk outside the stored range.
-        $id    = (int) $id;
-        $start = max(0, (int) $start);
-        $filter = (int) $filter;
+        // Read each parameter explicitly rather than through a variable-variable loop, which
+        // turned any key later added to the defaults into a local controlled by the caller
+        // ($params is $_GET). getFromDB() does not cast its argument, and MySQL coerces a
+        // string to a number when comparing it to an INT column: "1-alert(1)" would match
+        // the row with id 1 and then reach the template. $start is also used in pagination
+        // arithmetic, which raises a TypeError in PHP 8 on a non numeric value and lets a
+        // negative offset walk outside the stored range.
+        $id     = (int) ($params['id'] ?? 0);
+        $start  = max(0, (int) ($params['start'] ?? 0));
+        $filter = (int) ($params['filter'] ?? 0);
 
         if (!$this->getFromDB($id)) {
             TemplateRenderer::getInstance()->display('@addressing/report_invalid.html.twig');

src/IpComment.php

@@ -43,4 +43,36 @@ public static function getTypeName($nb = 0)
 
         return _n('IP Addressing', 'IP Addressing', $nb, 'addressing');
     }
+
+    /*
+     * The table has no entities_id, so checkEntity() is a no-op and can() would reduce to the
+     * global plugin_addressing right: the generic front/ipComment.form.php route of the core
+     * would then read, rewrite or purge the comments of any entity's ranges. Comments are only
+     * written by ajax/ipcomment.php, which checks the parent range itself and never calls
+     * can() on this class, so no generic access is granted at all.
+     */
+    public static function canView(): bool
+    {
+        return false;
+    }
+
+    public static function canCreate(): bool
+    {
+        return false;
+    }
+
+    public static function canUpdate(): bool
+    {
+        return false;
+    }
+
+    public static function canDelete(): bool
+    {
+        return false;
+    }
+
+    public static function canPurge(): bool
+    {
+        return false;
+    }
 }

src/PingInfo.php

@@ -278,6 +278,38 @@ public static function getTypeName($nb = 0)
         return _n('IP Addressing', 'IP Addressing', $nb, 'addressing');
     }
 
+    /*
+     * The table has no entities_id, so checkEntity() is a no-op and can() would reduce to the
+     * global plugin_addressing right: the generic front/pingInfo.form.php route of the core
+     * would then read, rewrite or purge the ping results of any entity's ranges. Rows are only
+     * written by the cron, ajax/ping.php and Addressing::cleanDBonPurge(), none of which goes
+     * through can() on this class, so no generic access is granted at all.
+     */
+    public static function canView(): bool
+    {
+        return false;
+    }
+
+    public static function canCreate(): bool
+    {
+        return false;
+    }
+
+    public static function canUpdate(): bool
+    {
+        return false;
+    }
+
+    public static function canDelete(): bool
+    {
+        return false;
+    }
+
+    public static function canPurge(): bool
+    {
+        return false;
+    }
+
     /**
      * @param $name
      **/