detect CWE-113 with sink javax/servlet/http/HttpServletResponse.setHeader

[bradflood]

This issue is similar to #341. It was found by IBM AppScan, which we are abandoning in favor of find-sec-bugs. The code below is notional, as I have not verified it. If I have time this weekend, I can submit a PR for this and #341.

package testcode.xss.servlets;

import org.apache.commons.lang.StringEscapeUtils;
import org.owasp.esapi.ESAPI;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;

public class XssServlet7 extends HttpServlet {
	/*
 	 * source=java.io.FileInputStream.read(byte[]):int
	 * sink=javax.servlet.http.HttpServletResponse.setHeader(java.lang.String;java.lang.String):void
	 *
	 * possible solution: add to https://github.com/find-sec-bugs/find-sec-bugs/blob/master/plugin/src/main/resources/injection-sinks/xss-servlet.txt
	 * (needs verification): javax/servlet/http/HttpServletResponse.setHeader (ILjava/lang/String;ILjava/lang/String)V:0 
 	 */
    protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
    	File srcFile = new File("example.txt");
	FileInputStream fis = new FileInputStream(srcFile);
	
	byte b[] = new byte[1024];
	in.read(b);
	String zipFileName = "Example_Report_" + b + ".zip";
	response.setHeader("Content-Disposition", "attachment; filename=" + zipFileName);
	
    }
}

[h3xstream]

I left #341 open on purpose.. because it is really easy to implement and I was hoping for some hacktoberfest momentum to get new contributors.

Header splitting should be already covered :

find-sec-bugs/plugin/src/main/resources/injection-sinks/response-splitting.txt

Lines 3 to 7 in 89182fb

	javax/servlet/http/HttpServletResponse.addHeader(Ljava/lang/String;Ljava/lang/String;)V:0,1
	javax/servlet/http/HttpServletResponse.setHeader(Ljava/lang/String;Ljava/lang/String;)V:0,1
	javax/servlet/http/HttpServletResponseWrapper.addHeader(Ljava/lang/String;Ljava/lang/String;)V:0,1
	javax/servlet/http/HttpServletResponseWrapper.setHeader(Ljava/lang/String;Ljava/lang/String;)V:0,1

[bradflood]

@h3xstream I can take both #341 and #354.

[h3xstream]

@bradflood Cool 👍

[bradflood]

Verified comment from @h3xstream that this issue appears to be covered by existing response splitting checks.

My client is using find-sec-bugs 1.6, sonar-findbugs 3.6, and it's possible this was fixed after 1.6.

I would like to keep this open until a version of sonar-findbugs is released containing this fix. It appears this fix is slotted for find-sec-bugs 1.8

[h3xstream]

If we look at the test cases, we already have, it should be covered.
https://github.com/find-sec-bugs/find-sec-bugs/blob/master/plugin/src/test/java/testcode/ResponseSplittingServlet.java#L17
https://github.com/find-sec-bugs/find-sec-bugs/blob/master/plugin/src/test/java/testcode/ResponseSplittingServlet.java#L32

The issue might not get reported if you only show normal and high priority. When FSB can't find the source, it will reported as low. Header injection is almost a vulnerability of the past, the likelyhood that your web server is vulnerable to header injection is low.
https://github.com/find-sec-bugs/find-sec-bugs/blob/master/plugin/src/main/java/com/h3xstream/findsecbugs/injection/crlf/CrlfLogInjectionDetector.java#L38-L55

[h3xstream]

I will open a new ticket to verify if FileInputStream.read(X) is tainting the parameter X.
https://github.com/find-sec-bugs/find-sec-bugs/search?l=Text&q=%22Stream.read%22

[h3xstream]

Ref #461