hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix XSS FPs when content type is safe

Browse Source
This commit is contained in:
Owen Mansel-Chan
2025-01-27 14:37:14 +00:00
parent 9f3572d15a
commit 0ccf4cecb8
3 changed files with 108 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
java/ql/lib/semmle/code/java/frameworks/Servlets.qll
View File

@@ -315,6 +315,16 @@ class ResponseSetHeaderMethod extends Method {
}
}
/**
* The method `setContentType` declared in `javax.servlet.http.HttpServletResponse`.
*/
class ResponseSetContentTypeMethod extends Method {
ResponseSetContentTypeMethod() {
this.getDeclaringType() instanceof ServletResponse and
this.hasName("setContentType")
}
}
/**
* A class that has `javax.servlet.Servlet` as an ancestor.
*/

27
java/ql/lib/semmle/code/java/security/XSS.qll
View File

@@ -92,9 +92,25 @@ private class WritingMethod extends Method {
/** An output stream or writer that writes to a servlet, JSP or JSF response. */
class XssVulnerableWriterSource extends MethodCall {
XssVulnerableWriterSource() {
this.getMethod() instanceof ServletResponseGetWriterMethod
or
this.getMethod() instanceof ServletResponseGetOutputStreamMethod
(
this.getMethod() instanceof ServletResponseGetWriterMethod
or
this.getMethod() instanceof ServletResponseGetOutputStreamMethod
) and
not exists(MethodCall mc, Expr contentType |
mc.getMethod() instanceof ResponseSetContentTypeMethod and
contentType = mc.getArgument(0)
or
(
mc.getMethod() instanceof ResponseAddHeaderMethod or
mc.getMethod() instanceof ResponseSetHeaderMethod
) and
mc.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() = "content-type" and
contentType = mc.getArgument(1)
|
isXssSafeContentTypeString(contentType.(CompileTimeConstantExpr).getStringValue()) and
DataFlow::localExprFlow(mc.getQualifier(), this.getQualifier())
)
or
exists(Method m | m = this.getMethod() |
m.hasQualifiedName("javax.servlet.jsp", "JspContext", "getOut")
@@ -106,6 +122,11 @@ class XssVulnerableWriterSource extends MethodCall {
}
}
pragma[nomagic]
private predicate isXssSafeContentTypeString(string s) {
s = any(CompileTimeConstantExpr cte).getStringValue() and isXssSafeContentType(s)
}
/**
* A xss vulnerable writer source node.
*/

75
java/ql/test/query-tests/security/CWE-079/semmle/tests/XSS.java
View File

@@ -12,7 +12,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
public class XSS extends HttpServlet {
protected void doGet(HttpServletRequest request, HttpServletResponse response)
protected void doGet(HttpServletRequest request, HttpServletResponse response, boolean safeContentType, boolean getWriter, int setContentMethod)
throws ServletException, IOException {
// BAD: a request parameter is written directly to the Servlet response stream
response.getWriter()
@@ -38,6 +38,79 @@ public class XSS extends HttpServlet {
// GOOD: sanitizer
response.getOutputStream().write(hudson.Util.escape(request.getPathInfo()).getBytes()); // safe
if(safeContentType) {
if(getWriter) {
if(setContentMethod == 0) {
// GOOD: set content-type to something safe
response.setContentType("text/plain");
response.getWriter().print(request.getPathInfo());
}
else if(setContentMethod == 1) {
// GOOD: set content-type to something safe
response.setHeader("Content-Type", "text/plain");
response.getWriter().print(request.getPathInfo());
}
else {
// GOOD: set content-type to something safe
response.addHeader("Content-Type", "text/plain");
response.getWriter().print(request.getPathInfo());
}
}
else {
if(setContentMethod == 0) {
// GOOD: set content-type to something safe
response.setContentType("text/plain");
response.getOutputStream().write(request.getPathInfo().getBytes());
}
else if(setContentMethod == 1) {
// GOOD: set content-type to something safe
response.setHeader("Content-Type", "text/plain");
response.getOutputStream().write(request.getPathInfo().getBytes());
}
else {
// GOOD: set content-type to something safe
response.addHeader("Content-Type", "text/plain");
response.getOutputStream().write(request.getPathInfo().getBytes());
}
}
}
else {
if(getWriter) {
if(setContentMethod == 0) {
// BAD: set content-type to something that is not safe
response.setContentType("text/html");
response.getWriter().print(request.getPathInfo()); // $ xss
}
else if(setContentMethod == 1) {
// BAD: set content-type to something that is not safe
response.setHeader("Content-Type", "text/html");
response.getWriter().print(request.getPathInfo()); // $ xss
}
else {
// BAD: set content-type to something that is not safe
response.addHeader("Content-Type", "text/html");
response.getWriter().print(request.getPathInfo()); // $ xss
}
}
else {
if(setContentMethod == 0) {
// BAD: set content-type to something that is not safe
response.setContentType("text/html");
response.getOutputStream().write(request.getPathInfo().getBytes()); // $ xss
}
else if(setContentMethod == 1) {
// BAD: set content-type to something that is not safe
response.setHeader("Content-Type", "text/html");
response.getOutputStream().write(request.getPathInfo().getBytes()); // $ xss
}
else {
// BAD: set content-type to something that is not safe
response.addHeader("Content-Type", "text/html");
response.getOutputStream().write(request.getPathInfo().getBytes()); // $ xss
}
}
}
}
/**