Sanitize/validate variable to avoid cross-site-scripting attack

Question

I get this issue with CheckMarx security scan:

Method exec at line 69 of web\src\main\java\abc\web\actions\HomeAction.java gets user input for the CNF_KEY_COSN element. This element’s value then flows through the code without being properly sanitized or validated and is eventually displayed to the user in method logException at line 905 of web\src\main\java\gov\abc\external\info\ServiceHelper.java. This may enable a Cross-Site-Scripting attack.

Line 69 of HomeAction.java:

String cosn = (String) request.getParameter(CNF_KEY_CON);

Line 905 in ServiceHelper.java just logs the error:

private static void logException(InfoServiceException exception, String message) {
    String newMessage = message + ": " + exception.getMessageForLogging();
    try {
        log.error(newMessage, exception);
    } catch (Exception e) {
        // fallback to console
        System.out.println("error logging exception ->");
        e.printStackTrace(System.out);
        System.out.println("exception ->");
        System.out.print(newMessage);
        if (exception != null) exception.printStackTrace(System.out);
    }
}

Changed another block of code in HomeAction.java to:

if(cosn!= null && cosn.matches("[0-9a-zA-Z_]+")) {
  ...
}

But that didn't help. How do I validate/sanitize/encode Line 69. Any help is much appreciated.

Thanks

As Checkmarx says, there is a flow from `cosn` to `logException` (but you say the method is `log.error`). You saying that you added sanitation in `HomeAction.java` but we don't know if this sanitation is in the vulnerable flow. — baruchiro, May 08 '20 at 06:52
@baruchiro Sorry for not adding the complete code for the method. Edited the post. Thanks — Harry, May 08 '20 at 13:32

score 2 · Answer 1 · answered May 08 '20 at 13:47

2

You can sanitise strings for XSS attacks using Jsoup there is a clean() method for this. You would do something like this to sanitise the input:

String sanitizedInput = Jsoup.clean(originalInput, "", Whitelist.none(), new OutputSettings().prettyPrint(false));

answered May 08 '20 at 13:47

A.Stern

83
6

Stern, thank you. I made the following changes but still no luck......... String sanitizedCosn = null; if (cosn != null) { sanitizedCosn = Jsoup.clean(cosn, "", Whitelist.none(), new OutputSettings().prettyPrint(false)); } – Harry May 08 '20 at 16:38
One more edit: String cosn = (String) Jsoup.clean(request.getParameter(CNF_KEY_COSN), "", Whitelist.none(), new OutputSettings().prettyPrint(false)); Still same error – Harry May 08 '20 at 17:33
So, when you say that it produced the same error, you mean that you sent something like "" and it appeared the same way in the exception message? – A.Stern May 09 '20 at 13:24
No, CheckMarx (code analyzer) still reports it as a high security risk. The same error message as in the post. Thanks – Harry May 09 '20 at 16:25

score 1 · Answer 2 · answered May 14 '20 at 09:30

Checkmarx defines a set of sanitizers that you can check in the system.

Based on your source code snippets; i assume that; i) you are appending 'cosn' to 'message' ii) application is web-based in nature (in view of the request.getParameter) iii) message is been displayed to the console or log to a file.

You could consider using Google Guava or Apache Commons Test to html escape the input.

import com.google.common.html.HtmlEscapers;

public void testGuavaHtmlEscapers(){
    String badInput = "<script> alert me! <script>";
    String escapedLocation = HtmlEscapers.htmlEscaper().escape(badInput);
    System.out.println("<h1> Location: " + escapedLocation + "<h1>");
}

import static org.apache.commons.text.StringEscapeUtils.escapeHtml4;

public void testHtmlEscapers(){
    String badInput = "<script> alert me! <script>";
    System.out.println(escapeHtml4(badInput));
}

I would also consider if there is sensitive information, that i should mask e.g., using String.replace.

public void testReplace(){
    String email = "some-email@domail.com";
    String masked = email.replaceAll("(?<=.).(?=[^@]*?.@)", "*");
    System.out.println(masked);
}

Above 3 sanitization methods will work similarly.

score 0 · Answer 3 · answered May 18 '20 at 01:25

This is likely a false positive (technically, "not exploitable" in Checkmarx) with regard to XSS, depending on how you process and display logs. If logs are ever displayed in a browser as html, it might be vulnerable to blind XSS from this applications point of view, but it would be a vulnerability in whatever component displays logs as html, and not in the code above.

Contrary to other answers, you should not encode the message here. Whatever technology you use for logging will of course have to encode it properly for its own use (like for example if it's stored as JSON, data will have to be JSON-encoded), but that has nothing to do with XSS, or with this problem at all.

This is just raw data, and you can store raw data as is. If you encode it here, you will have a hard time displaying it in any other way. For example if you apply html encoding, you can only display it in html (or you have to decode, which will negate any effect). It doesn't make sense. XSS would arise if you displayed these logs in a browser - in which case whatever displays it would have to encode it properly, but that's not the case here.

Note though that it can still be a log injection vulnerability. Make sure that whatever way you store logs, that log store **does* apply necessary encoding. If it's a text file, you probably want to remove newlines so that fake lines cannot be added to the log. If it's json, you will want to encode to json, and so on. But that's a feature of your log facility, and not the code above.

Sanitize/validate variable to avoid cross-site-scripting attack

3 Answers3