Stored xss security threat

Question

We are working on security prevention and in the below code at buffer is giving stored XSS attack...below is the info we are getting from Checkmark tool.

Method test gets data from the database, for the buffer element. This element’s value then flows through the code without being properly filtered or encoded and is eventually displayed to the user in method test. This may enable a Stored Cross-Site-Scripting attack.

private void test(HttpServletResponse response, SessionInfo sessionInfo, String applicationName, String resourceName, String resourcePath, String domainAddress, String siteAddress, String fileNames) throws KatalystServletException, IOException, FSException
{
    InputStream in = resourceFile.getInputStream();
    ZipOutputStream zipOut = null;
    try {
        byte[] buffer = new byte[8 * 1024];
        int bytesRead = 0;
        while ((bytesRead = in.read(buffer)) != -1) {
            zipOut.write(buffer, 0, bytesRead);
        }
    } finally {
        in.close();
        zipOut.flush();
    }
}

Answer 1

Here is the solution for the question and also for HRA_JAVA_CGI_STORED_XSS or Stored XSS Vulnerability flagged by Checkmarx in Java.

This issue is flagged by Checkmarx for any Java File Operations that use a byte[] array as a buffer.

You need to sanitize your hands(sad face) and also your byte array before writing to any output stream. You can use ESAPI's validator function to remediate this.

Add the ESAPI jar to your project

   <dependency>
        <groupId>org.owasp.esapi</groupId>
        <artifactId>esapi</artifactId>
        <version>2.2.2.0</version>
    </dependency>

import org.owasp.esapi.*;

Before writing into the OutputStream ,in your case a ZipOutputStream we need to filter the byte array buffer using getValidFileContent(String context,byte[] input, int maxBytes, boolean allowNull)

This method checks for any end of file exploits, File Corruption attacks or other intrusion attacks(I could not find much info regarding them, will update if I find more info.)

https://javadoc.io/doc/org.owasp.esapi/esapi/2.2.2.0/org/owasp/esapi/Validator.html#getValidFileContent-java.lang.String-byte:A-int-boolean-

private void test(HttpServletResponse response, SessionInfo sessionInfo, String applicationName, String resourceName, String resourcePath, String domainAddress, String siteAddress, String fileNames) throws KatalystServletException, IOException, FSException
{
    InputStream in = resourceFile.getInputStream();
    ZipOutputStream zipOut = null;
    try {
        byte[] buffer = new byte[8 * 1024];
        int bytesRead = 0;
        while ((bytesRead = in.read(buffer)) != -1) {

      // HERE IS THE MAGIC LINE
      buffer=ESAPI.validator().getValidFileContent("blah blah",buffer,8192,true);
      //TADA

            zipOut.write(buffer, 0, bytesRead);
        }
    } finally {
        in.close();
        zipOut.flush();
    }
}

I had similar issue with a Util method and it was flagged at 8 places. All of them were resolved after this fix.

I would also recommend using try catch finally block and closing the InputStreams and OutputStreams in the 'finally' section or you could use 'Try with Resources'.

Not Closing these streams was flagged as 'Resource Starvation' in another CodeScan- Unreleased resources could cause system slowdowns and starve other components of memory.