Sanitize query strings before logging

See: https://sonarcloud.io/organizations/bnc-projects/rules?open=javasecurity%3AS5145&rule_key=javasecurity%3AS5145

Author: james

service/src/main/java/com/bnc/sbjb/rest/DefaultExceptionHandler.java

@@ -86,8 +86,27 @@ public CustomError handleExceptionMethodNotAllowed(final Exception ex) {
     @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
     @ResponseBody
     public CustomError handleException(HttpServletRequest httpServletRequest, Exception ex) {
-        logger.error("Unknown exception [queryString=\"{}\" errorMessage=\"{}\"]", httpServletRequest.getQueryString(), ex.getMessage(), ex);
+        final String queryString = sanitizedQueryString(httpServletRequest.getQueryString(), 250);
+        logger.error("Unknown exception [queryString=\"{}\" errorMessage=\"{}\"]", queryString, ex.getMessage(), ex);
         return new CustomError(HttpStatus.INTERNAL_SERVER_ERROR,
             "Oops something went wrong. Please contact us if this keeps occurring.");
     }
+
+    /**
+     * Logging should not be vulnerable to injection attacks.
+     *
+     * @param queryString The query string to sanitize.
+     * @param maxLength The max acceptable length
+     * @return A string that is no more characters than the max length requested, with new lines and tabs replaced.
+     */
+    private String sanitizedQueryString(String queryString, int maxLength) {
+        if (queryString == null) {
+            return "";
+        }
+        return queryString
+            .substring(0, Math.min(queryString.length(), maxLength))
+            .replaceAll("[\n]+", "\\\\n")
+            .replaceAll("[\r]+", "\\\\r")
+            .replaceAll("[\t]+", "\\\\t");
+    }
 }

service/src/test/java/com/bnc/sbjb/rest/DefaultExceptionHandlerTest.java

@@ -0,0 +1,46 @@
+package com.bnc.sbjb.rest;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import javax.servlet.http.HttpServletRequest;
+import org.junit.jupiter.api.Test;
+
+class DefaultExceptionHandlerTest {
+
+    @Test
+    void testHandleExceptionWithBigQueryString() {
+        final HttpServletRequest mockRequest = mock(HttpServletRequest.class);
+        when(mockRequest.getQueryString()).thenReturn(
+            "This is a string with a lot of text and several extra whitespaces\n"
+                + "This is a string with a lot of text and several extra whitespaces\n"
+                + "This is a string with a lot of text and several extra whitespaces\n"
+                + "This is a string with a lot of text and several extra whitespaces\n"
+                + "This is a string with a lot of text and several extra whitespaces\n");
+        new DefaultExceptionHandler().handleException(mockRequest, new Exception("Just testing"));
+        // Just to include a test.
+        verify(mockRequest).getQueryString();
+        verifyNoMoreInteractions(mockRequest);
+    }
+
+    @Test
+    void testHandleExceptionWithLittleQueryString() {
+        final HttpServletRequest mockRequest = mock(HttpServletRequest.class);
+        when(mockRequest.getQueryString()).thenReturn("\n");
+        new DefaultExceptionHandler().handleException(mockRequest, new Exception("Just testing"));
+        // Just to include a test.
+        verify(mockRequest).getQueryString();
+        verifyNoMoreInteractions(mockRequest);
+    }
+
+    @Test
+    void testHandleExceptionWithNoQueryString() {
+        final HttpServletRequest mockRequest = mock(HttpServletRequest.class);
+        when(mockRequest.getQueryString()).thenReturn(null);
+        new DefaultExceptionHandler().handleException(mockRequest, new Exception("Just testing"));
+        // Just to include a test.
+        verify(mockRequest).getQueryString();
+    }
+}