Repository: librenms/librenms (language: php)

Issue:
Fixed XSS display issues across hostname popup
I've had a click through the webui after these changes and I haven't seen anything broken afterwards.

DO NOT DELETE THE UNDERLYING TEXT

#### Please note

> Please read this information carefully. You can run `./lnms dev:check` to check your code before submitting.

- [ ] Have you followed our [code guidelines?](https://docs.librenms.org/Developing/Code-Guidelines/)
- [ ] If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
- [ ] If my Pull Request makes discovery/polling/yaml changes, I have added/updated [test data](https://docs.librenms.org/Developing/os/Test-Units/).

#### Testers

If you would like to test this pull request then please run: `./scripts/github-apply <pr_id>`, i.e `./scripts/github-apply 5926`
After you are done testing, you can remove the changes with `./scripts/github-remove`.  If there are schema changes, you can ask on discord how to revert.



Discussion / hints:

Fix patch:
diff --git a/LibreNMS/Util/Url.php b/LibreNMS/Util/Url.php
index bf6912231fc7..6dfbb7f8502e 100644
--- a/LibreNMS/Util/Url.php
+++ b/LibreNMS/Util/Url.php
@@ -51,11 +51,11 @@ class Url
     public static function deviceLink($device, $text = '', $vars = [], $start = 0, $end = 0, $escape_text = 1, $overlib = 1)
     {
         if (! $device instanceof Device || ! $device->hostname) {
-            return (string) $text;
+            return $escape_text ? htmlentities($text) : (string) $text;
         }
 
         if (! $device->canAccess(Auth::user())) {
-            return $device->displayName();
+            return $escape_text ? htmlentities($device->displayName()) : $device->displayName();
         }
 
         if (! $start) {
@@ -79,30 +79,31 @@ public static function deviceLink($device, $text = '', $vars = [], $start = 0, $
         $url = Url::deviceUrl($device, $vars);
 
         // beginning of overlib box contains large hostname followed by hardware & OS details
-        $contents = '<div><span class="list-large">' . $device->displayName() . '</span>';
+        // because we are injecting this into javascript htmlentities alone won't work, so strip_tags too
+        $contents = '<div><span class="list-large">' . htmlentities(strip_tags($device->displayName())) . '</span>';
         $devinfo = '';
         if ($device->hardware) {
-            $devinfo .= htmlentities($device->hardware);
+            $devinfo .= $device->hardware;
         }
 
         if ($device->os) {
-            $devinfo .= ($devinfo ? ' - ' : '') . htmlentities(Config::getOsSetting($device->os, 'text'));
+            $devinfo .= ($devinfo ? ' - ' : '') . Config::getOsSetting($device->os, 'text');
         }
 
         if ($device->version) {
-            $devinfo .= ($devinfo ? ' - ' : '') . htmlentities($device->version);
+            $devinfo .= ($devinfo ? ' - ' : '') . $device->version;
         }
 
         if ($device->features) {
-            $devinfo .= ' (' . htmlentities($device->features) . ')';
+            $devinfo .= ' (' . $device->features . ')';
         }
 
         if ($devinfo) {
-            $contents .= '<br />' . $devinfo;
+            $contents .= '<br />' . htmlentities(strip_tags($devinfo));
         }
 
         if ($device->location_id) {
-            $contents .= '<br />' . htmlentities($device->location ?? '');
+            $contents .= '<br />' . htmlentities(strip_tags($device->location ?? ''));
         }
 
         $contents .= '</div><br />';
diff --git a/includes/html/common/availability-map.inc.php b/includes/html/common/availability-map.inc.php
index 6c63a36c548e..1c7a3f451506 100644
--- a/includes/html/common/availability-map.inc.php
+++ b/includes/html/common/availability-map.inc.php
@@ -231,7 +231,7 @@
                 $host_maintenance_count++;
             }
 
-            $device_system_name = format_hostname($device);
+            $device_system_name = htmlentities(format_hostname($device));
 
             if (Config::get('webui.availability_map_compact') == 0) {
                 if ($directpage == 'yes') {
@@ -287,33 +287,34 @@
                     $serviceState = 'down';
                     $service_down_count++;
                 }
-                $service_system_name = format_hostname($service);
+                $device_name = format_hostname($service);
+                $service_descr = " - ${service['service_type']} - ${service['service_desc']}";
 
                 if (Config::get('webui.availability_map_compact') == 0) {
                     if ($directpage == 'yes') {
                         $deviceIcon = getIconTag($service);
                         $temp_output[] = '
-                        <a href="' . \LibreNMS\Util\Url::generate(['page' => 'device', 'device' => $service['device_id'], 'tab' => 'services']) . '" title="' . $service_system_name . ' - ' . $service['service_type'] . ' - ' . $service['service_desc'] . '">
+                        <a href="' . \LibreNMS\Util\Url::generate(['page' => 'device', 'device' => $service['device_id'], 'tab' => 'services']) . '" title="' . htmlentities($device_name . $service_descr) . '">
                             <div class="service-availability ' . $serviceState . '" style="width:' . Config::get('webui.availability_map_box_size') . 'px;">
                                 <span class="service-name-label label ' . $serviceLabel . ' label-font-border">' . $service['service_type'] . '</span>
                                 <span class="availability-label label ' . $serviceLabel . ' label-font-border">' . $serviceState . '</span>
                                 <span class="device-icon">' . $deviceIcon . '</span><br>
-                                <span class="small">' . shorthost($service_system_name) . '</span>
+                                <span class="small">' . htmlentities(shorthost($device_name)) . '</span>
                             </div>
                         </a>';
                     } else {
-                        $serviceText = $service['service_type'] . ' - ' . $serviceState;
+                        $serviceText = htmlentities($service['service_type']) . ' - ' . $serviceState;
                         if ($settings['color_only_select'] == 1) {
                             $serviceText = ' ';
                             $serviceLabel .= ' widget-availability-fixed';
                         }
                         $temp_output[] = '
-                        <a href="' . \LibreNMS\Util\Url::generate(['page' => 'device', 'device' => $service['device_id'], 'tab' => 'services']) . '" title="' . shorthost($service_system_name) . ' - ' . $service['service_type'] . ' - ' . $service['service_desc'] . '">
+                        <a href="' . \LibreNMS\Util\Url::generate(['page' => 'device', 'device' => $service['device_id'], 'tab' => 'services']) . '" title="' . htmlentities(shorthost($device_name) . $service_descr) . '">
                             <span class="label ' . $serviceLabel . ' widget-availability label-font-border">' . $serviceText . '</span>
                         </a>';
                     }
                 } else {
-                    $temp_output[] = "<a href='" . \LibreNMS\Util\Url::generate(['page' => 'device', 'device' => $service['device_id'], 'tab' => 'services']) . "' title='${service_system_name} - ${service['service_type']} - ${service['service_desc']}'><div class='" . $serviceLabelOld . "' style='width:${compact_tile}px;height:${compact_tile}px;'></div></a>";
+                    $temp_output[] = "<a href='" . \LibreNMS\Util\Url::generate(['page' => 'device', 'device' => $service['device_id'], 'tab' => 'services']) . "' title='$device_name$service_descr'><div class='" . $serviceLabelOld . "' style='width:${compact_tile}px;height:${compact_tile}px;'></div></a>";
                 }
             }
         } else {
diff --git a/includes/html/pages/edituser.inc.php b/includes/html/pages/edituser.inc.php
index 9efd2404e608..884584cdcd4b 100644
--- a/includes/html/pages/edituser.inc.php
+++ b/includes/html/pages/edituser.inc.php
@@ -80,7 +80,7 @@
 
         $device_perms = dbFetchRows('SELECT * from devices_perms as P, devices as D WHERE `user_id` = ? AND D.device_id = P.device_id', [$user_data['user_id']]);
         foreach ($device_perms as $device_perm) {
-            echo '<tr><td><strong>' . format_hostname($device_perm) . "</td><td> <a href='edituser/action=deldevperm/user_id=" . $vars['user_id'] . '/device_id=' . $device_perm['device_id'] . "'><i class='fa fa-trash fa-lg icon-theme' aria-hidden='true'></i></a></strong></td></tr>";
+            echo '<tr><td><strong>' . htmlentities(format_hostname($device_perm)) . "</td><td> <a href='edituser/action=deldevperm/user_id=" . $vars['user_id'] . '/device_id=' . $device_perm['device_id'] . "'><i class='fa fa-trash fa-lg icon-theme' aria-hidden='true'></i></a></strong></td></tr>";
             $access_list[] = $device_perm['device_id'];
             $permdone = 'yes';
         }


Test patch: