1

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

2
  • 1
    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 8, 2020 at 6:52
  • @baruchiro Sorry for not adding the complete code for the method. Edited the post. Thanks
    – Harry
    May 8, 2020 at 13:32

4 Answers 4

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));
4
  • 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 8, 2020 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 8, 2020 at 17:33
  • So, when you say that it produced the same error, you mean that you sent something like "<script>alert('\test')</script>" and it appeared the same way in the exception message?
    – A.Stern
    May 9, 2020 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 9, 2020 at 16:25
1

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 = "[email protected]";
    String masked = email.replaceAll("(?<=.).(?=[^@]*?.@)", "*");
    System.out.println(masked);
}

Above 3 sanitization methods will work similarly.

0

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.

-1

Encode.forHtml() worked for me

1
  • Your answer could be improved with additional supporting information. Please edit to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers in the help center.
    – Community Bot
    Oct 24, 2023 at 10:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.