Harry
Harry

Reputation: 548

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

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

Upvotes: 1

Views: 32071

Answers (4)

Влад Банар
Влад Банар

Reputation: 1

Encode.forHtml() worked for me

Upvotes: -1

Gabor Lengyel
Gabor Lengyel

Reputation: 15599

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.

Upvotes: 1

mystery
mystery

Reputation: 37

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.

Upvotes: 1

A.Stern
A.Stern

Reputation: 103

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));

Upvotes: 3

Related Questions