diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cae3bd45..523269684 100644 --- 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/FROST-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/parser/path/PathParser.java b/FROST-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/parser/path/PathParser.java index cf4cc2c61..ff293be52 100644 --- a/FROST-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/frostserver/parser/path/PathParser.java +++ b/FROST-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 index 6187c4151..a7f679474 100644 --- 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 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 index 442cc316b..1712a1ced 100644 --- 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