From dfb949c79c388652a76f05e8f8f8ef44b518e7c8 Mon Sep 17 00:00:00 2001
From: Virtually Nick <vnick@apache.org>
Date: Thu, 3 Oct 2024 14:15:37 -0400
Subject: [PATCH] GUACAMOLE-1020: Perform some additional null checks in
 restriction verification service.

---
 .../RestrictionVerificationService.java       | 45 +++++++++++++++----
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/auth/restrict/RestrictionVerificationService.java b/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/auth/restrict/RestrictionVerificationService.java
index c0666cd7db..e84e14f528 100644
--- a/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/auth/restrict/RestrictionVerificationService.java
+++ b/extensions/guacamole-auth-restrict/src/main/java/org/apache/guacamole/auth/restrict/RestrictionVerificationService.java
@@ -135,9 +135,6 @@ public static RestrictionType allowedByTimeRestrictions(String allowedTimeString
     public static RestrictionType allowedByHostRestrictions(String allowedHostsString,
             String deniedHostsString, String remoteAddress) {
         
-        // Convert the string to a HostName
-        HostName remoteHostName = new HostName(remoteAddress);
-        
         // If attributes do not exist or are empty then the action is allowed.
         if ((allowedHostsString == null || allowedHostsString.isEmpty()) 
                 && (deniedHostsString == null || deniedHostsString.isEmpty()))
@@ -152,19 +149,42 @@ public static RestrictionType allowedByHostRestrictions(String allowedHostsStrin
             return RestrictionType.IMPLICIT_DENY;
         }
         
+        // Convert the string to a HostName
+        HostName remoteHostName = new HostName(remoteAddress);
+
+        // If for some reason we end up with a NULL value, here, something has gone wrong
+        // in attempting to convert the String form of the remote address into the
+        // HostName object, so we bail out and deny the action.
+        if (remoteHostName == null) {
+            LOGGER.error("Unable to parse remote address, action will be denied.");
+            LOGGER.debug("Error parsing \"{}\" into a valid hostname or IP address.", remoteAddress);
+            return RestrictionType.IMPLICIT_DENY;
+        }
+        
         // Split denied hosts attribute and process each entry, checking them
-        // against the current remote address, and returning false if a match is
-        // found.
+        // against the current remote address, and returning a deny restriction
+        // if a match is found, or if an error occurs in processing a host in
+        // the list.
         List<HostName> deniedHosts = HostRestrictionParser.parseHostList(deniedHostsString);
         for (HostName hostName : deniedHosts) {
+
+            if (hostName == null) {
+                LOGGER.warn("Host name is null, denying this action.");
+                LOGGER.debug("Received NULL host object, returning an implicit deny.");
+                return RestrictionType.IMPLICIT_DENY;
+            }
+
             try {
-                if (hostName.isAddress() && hostName.toAddress().contains(remoteHostName.asAddress()))
+                if (hostName.isAddress()
+                        && hostName.toAddress().contains(remoteHostName.asAddress())) {
                     return RestrictionType.EXPLICIT_DENY;
+                }
 
-                else
+                else {
                     for (IPAddress currAddr : hostName.toAllAddresses())
-                        if (currAddr.matches(remoteHostName.asAddressString()))
+                        if (currAddr != null && currAddr.matches(remoteHostName.asAddressString()))
                             return RestrictionType.EXPLICIT_DENY;
+                }
             }
             catch (UnknownHostException | HostNameException e) {
                 LOGGER.warn("Unknown or invalid host in denied hosts list: \"{}\"", hostName);
@@ -182,6 +202,13 @@ public static RestrictionType allowedByHostRestrictions(String allowedHostsStrin
         // action if there are any matches.
         List<HostName> allowedHosts = HostRestrictionParser.parseHostList(allowedHostsString);
         for (HostName hostName : allowedHosts) {
+
+            if (hostName == null) {
+                LOGGER.warn("Host name from allow list is null, skipping and moving to the next.");
+                LOGGER.debug("Receive a null value from the list of allowed hosts, skipping the entry.");
+                continue;
+            }
+
             try {
                 // If the entry is an IP or Subnet, check the remote address against it directly
                 if (hostName.isAddress() && hostName.toAddress().contains(remoteHostName.asAddress()))
@@ -189,7 +216,7 @@ public static RestrictionType allowedByHostRestrictions(String allowedHostsStrin
                 
                 // Entry is a hostname, so resolve to IPs and check each one
                 for (IPAddress currAddr : hostName.toAllAddresses())
-                    if (currAddr.matches(remoteHostName.asAddressString()))
+                    if (currAddr != null && currAddr.matches(remoteHostName.asAddressString()))
                         return RestrictionType.EXPLICIT_ALLOW;
                 
             }