Fixed #1948: Sanitise user input in error output

FraunhoferIOSB · Jun 21, 2024 · cd9b926 · cd9b926
1 parent f631f5d
commit cd9b926
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 **Internal changes & Bugfixes**
 * [HELM] Added option to specify image pull secret name.
+* Fixed #1948: Sanitise user input in error output.
 
 
 ## Release version 2.3.2

diff --git a/...-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/parser/path/PathParser.java b/...-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/parser/path/PathParser.java
@@ -304,7 +304,7 @@ public static ResourcePath parsePath(ModelRegistry modelRegistry, String service
             PathParser pp = new PathParser(modelRegistry, resourcePath, user.isAdmin());
             pp.visit(parser.rootNode());
         } catch (ParseException ex) {
-            throw new IllegalArgumentException("Path " + StringHelper.cleanForLogging(path) + " is not valid: " + ex.getMessage());
+            throw new IllegalArgumentException("Path is not valid: " + ex.getMessage());
         }
         return resourcePath;
     }

diff --git a/FROST-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/service/Service.java b/FROST-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/service/Service.java
@@ -110,7 +110,6 @@ public class Service implements AutoCloseable {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(Service.class);
     private static final String EXCEPTION = "Exception:";
-    private static final String NOT_A_VALID_PATH = "Not a valid path";
     private static final String POST_ONLY_ALLOWED_TO_COLLECTIONS = "POST only allowed to Collections.";
     private static final String COULD_NOT_PARSE_JSON = "Could not parse json.";
     private static final String FAILED_TO_HANDLE_REQUEST_DETAILS_IN_DEBUG = "Failed to handle request (details in debug): {}";
@@ -375,8 +374,8 @@ private ServiceResponse handleGet(PersistenceManager pm, ServiceRequest request,
                     settings.getQueryDefaults().getServiceRootUrl(), version,
                     request.getUrlPath(),
                     request.getUserPrincipal());
-        } catch (IllegalArgumentException | IllegalStateException e) {
-            return errorResponse(response, 404, NOT_A_VALID_PATH + ": " + e.getMessage());
+        } catch (IllegalArgumentException | IllegalStateException ex) {
+            return errorResponse(response, 404, ex.getMessage());
         }
         Query query;
         ResultFormatter formatter;
@@ -463,8 +462,8 @@ private ServiceResponse handlePost(PersistenceManager pm, String urlPath, Servic
                     version,
                     urlPath,
                     request.getUserPrincipal());
-        } catch (IllegalArgumentException | IllegalStateException e) {
-            return errorResponse(response, 404, NOT_A_VALID_PATH + ": " + e.getMessage());
+        } catch (IllegalArgumentException | IllegalStateException ex) {
+            return errorResponse(response, 404, ex.getMessage());
         }
         if (!(path.getMainElement() instanceof PathElementEntitySet)) {
             return errorResponse(response, 400, POST_ONLY_ALLOWED_TO_COLLECTIONS);
@@ -635,8 +634,8 @@ private PathElementEntity parsePathForPutPatch(PersistenceManager pm, ServiceReq
                     request.getVersion(),
                     request.getUrlPath(),
                     request.getUserPrincipal());
-        } catch (IllegalArgumentException | IllegalStateException exc) {
-            throw new NoSuchEntityException(NOT_A_VALID_PATH + ": " + exc.getMessage());
+        } catch (IllegalArgumentException | IllegalStateException ex) {
+            throw new NoSuchEntityException(ex.getMessage());
         }
 
         if (!pm.validatePath(path)) {
@@ -728,8 +727,8 @@ private ServiceResponse executeDelete(ServiceRequest request, ServiceResponse re
                     request.getVersion(),
                     request.getUrlPath(),
                     request.getUserPrincipal());
-        } catch (IllegalArgumentException | IllegalStateException exc) {
-            return errorResponse(response, 404, NOT_A_VALID_PATH + ": " + exc.getMessage());
+        } catch (IllegalArgumentException | IllegalStateException ex) {
+            return errorResponse(response, 404, ex.getMessage());
         }
 
         if (path.isRef()) {
@@ -1017,16 +1016,17 @@ public static ServiceResponse errorResponse(ServiceResponse response, int code,
     }
 
     public static ServiceResponse jsonResponse(ServiceResponse response, String type, int code, String message) {
+        String cleanMessage = StringHelper.cleanForOutput(message, 200);
         Map<String, Object> body = new HashMap<>();
         body.put("type", type);
         body.put("code", code);
-        body.put("message", message);
+        body.put("message", cleanMessage);
         try {
             return response.setStatus(code, JsonWriter.writeObject(body));
         } catch (IOException ex) {
             LOGGER.error("Failed to serialise error response.", ex);
         }
-        return response.setStatus(code, message);
+        return response.setStatus(code, cleanMessage);
     }
 
 }
diff --git a/FROST-Server.Util/src/main/java/de/fraunhofer/iosb/ilt/frostserver/util/StringHelper.java b/FROST-Server.Util/src/main/java/de/fraunhofer/iosb/ilt/frostserver/util/StringHelper.java
@@ -52,6 +52,7 @@
 import net.time4j.format.expert.IsoDecimalStyle;
 import net.time4j.range.MomentInterval;
 import net.time4j.tz.ZonalOffset;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -63,6 +64,9 @@ public class StringHelper {
 
     public static final Charset UTF8 = StandardCharsets.UTF_8;
 
+    private static final String OUTPUT_CLEAN_REGEX = "[^A-Za-z0-9'.,;:()?/ _-]";
+    private static final String OUTPUT_CLEAN_REPLACE = " ";
+
     private static final Logger LOGGER = LoggerFactory.getLogger(StringHelper.class);
     private static final String UTF8_NOT_SUPPORTED = "UTF-8 not supported?";
     private static final NonZeroCondition NON_ZERO_FRACTION = new NonZeroCondition(PlainTime.NANO_OF_SECOND);
@@ -90,6 +94,36 @@ public static boolean isNullOrEmpty(Collection collection) {
         return collection == null || collection.isEmpty();
     }
 
+    /**
+     * Replaces characters that might cause problems in HTML and limits the
+     * length of the string. Currently everything that is not [A-Za-z0-9',;?/-]
+     * is changed to a ?.
+     *
+     * @param string The string to clean.
+     * @param maxLength The maximum length of string to return.
+     * @return The cleaned string.
+     */
+    public static String cleanForOutput(String string, int maxLength) {
+        if (string == null) {
+            return "null";
+        }
+        return cleanForOutput(StringUtils.truncate(string, maxLength));
+    }
+
+    /**
+     * Replaces characters that might cause problems in HTML. Currently
+     * everything that is not [A-Za-z0-9',;?/-] is changed to a ?.
+     *
+     * @param string The string to clean.
+     * @return The cleaned string.
+     */
+    public static String cleanForOutput(String string) {
+        if (string == null) {
+            return "null";
+        }
+        return string.replaceAll(OUTPUT_CLEAN_REGEX, OUTPUT_CLEAN_REPLACE);
+    }
+
     /**
      * Replaces characters that might break logging output. Currently: \n, \r
      * and \t