diff --git a/java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql b/java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql index 85464084135..02d28642659 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-598/SensitiveGetQuery.ql @@ -10,7 +10,6 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking -import semmle.code.java.frameworks.Servlets import semmle.code.java.security.SensitiveActions import DataFlow::PathGraph @@ -23,15 +22,15 @@ class SensitiveInfoExpr extends Expr { } } -/** Holds if `ma` is a method access to some override of `HttpServlet.doGet`. */ -private predicate isGetServletMethod(MethodAccess ma) { isServletMethod(ma, "doGet") } +/** Holds if `m` is a method of some override of `HttpServlet.doGet`. */ +private predicate isGetServletMethod(Method m) { isServletMethod(m) and m.getName() = "doGet" } /** Source of GET servlet requests. */ -class GetServletMethodSource extends DataFlow::ExprNode { - GetServletMethodSource() { - exists(MethodAccess ma | - isGetServletMethod(ma) and - ma = this.getExpr() +class GetHttpRequestSource extends DataFlow::ExprNode { + GetHttpRequestSource() { + exists(Method m | + isGetServletMethod(m) and + m.getParameter(0).getAnAccess() = this.asExpr() ) } } @@ -40,9 +39,15 @@ class GetServletMethodSource extends DataFlow::ExprNode { class SensitiveGetQueryConfiguration extends TaintTracking::Configuration { SensitiveGetQueryConfiguration() { this = "SensitiveGetQueryConfiguration" } - override predicate isSource(DataFlow::Node source) { source instanceof GetServletMethodSource } + override predicate isSource(DataFlow::Node source) { source instanceof GetHttpRequestSource } override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(MethodAccess ma | + isRequestGetParamMethod(ma) and pred.asExpr() = ma.getQualifier() and succ.asExpr() = ma + ) + } } from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveGetQueryConfiguration c diff --git a/java/ql/src/semmle/code/java/frameworks/Servlets.qll b/java/ql/src/semmle/code/java/frameworks/Servlets.qll index a7325608a3e..d6e59a1fb25 100644 --- a/java/ql/src/semmle/code/java/frameworks/Servlets.qll +++ b/java/ql/src/semmle/code/java/frameworks/Servlets.qll @@ -323,20 +323,17 @@ class ServletWebXMLListenerType extends RefType { } } -/** Holds if `ma` is a method access to some override of methods of `HttpServlet`, for example `doGet` or `doPost`. */ -predicate isServletMethod(MethodAccess ma, string methodName) { - exists(Method m | - m = ma.getEnclosingCallable() and - m.getDeclaringType() instanceof ServletClass and - m.getNumberOfParameters() = 2 and - m.getParameter(0).getType() instanceof ServletRequest and - m.getParameter(1).getType() instanceof ServletResponse and - m.getName() = methodName and - ma.getQualifier() = m.getParameter(0).getAnAccess() and - ( - ma.getMethod() instanceof ServletRequestGetParameterMethod or - ma.getMethod() instanceof ServletRequestGetParameterMapMethod or - ma.getMethod() instanceof HttpServletRequestGetQueryStringMethod - ) - ) +/** Holds if `m` is a request handler method (for example `doGet` or `doPost`). */ +predicate isServletMethod(Method m) { + m.getDeclaringType() instanceof ServletClass and + m.getNumberOfParameters() = 2 and + m.getParameter(0).getType() instanceof ServletRequest and + m.getParameter(1).getType() instanceof ServletResponse +} + +/** Holds if `ma` is a call that gets a request parameter. */ +predicate isRequestGetParamMethod(MethodAccess ma) { + ma.getMethod() instanceof ServletRequestGetParameterMethod or + ma.getMethod() instanceof ServletRequestGetParameterMapMethod or + ma.getMethod() instanceof HttpServletRequestGetQueryStringMethod } diff --git a/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery.expected b/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery.expected index 41394e6ca72..a6c4830740a 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery.expected @@ -1,18 +1,30 @@ edges -| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | -| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:30:14:48 | get(...) | -| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object | +| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | +| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:30:14:48 | get(...) | +| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object | | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | SensitiveGetQuery2.java:15:29:15:36 | password | | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | SensitiveGetQuery2.java:15:29:15:36 | password : Object | | SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | | SensitiveGetQuery2.java:15:29:15:36 | password : Object | SensitiveGetQuery2.java:18:40:18:54 | password : Object | | SensitiveGetQuery2.java:18:40:18:54 | password : Object | SensitiveGetQuery2.java:19:61:19:68 | password | +| SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | +| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | SensitiveGetQuery3.java:13:57:13:64 | password | +| SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | +| SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | +| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | +| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | +| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password | +| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password : String | +| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | +| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | +| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password | +| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password : String | | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password | | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password : String | | SensitiveGetQuery.java:14:29:14:36 | password : String | SensitiveGetQuery.java:17:40:17:54 | password : String | | SensitiveGetQuery.java:17:40:17:54 | password : String | SensitiveGetQuery.java:18:61:18:68 | password | nodes -| SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | semmle.label | getParameterMap(...) : Map | +| SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | semmle.label | request : HttpServletRequest | | SensitiveGetQuery2.java:14:21:14:48 | (...)... : Object | semmle.label | (...)... : Object | | SensitiveGetQuery2.java:14:30:14:48 | get(...) | semmle.label | get(...) | | SensitiveGetQuery2.java:14:30:14:48 | get(...) : Object | semmle.label | get(...) : Object | @@ -20,6 +32,13 @@ nodes | SensitiveGetQuery2.java:15:29:15:36 | password : Object | semmle.label | password : Object | | SensitiveGetQuery2.java:18:40:18:54 | password : Object | semmle.label | password : Object | | SensitiveGetQuery2.java:19:61:19:68 | password | semmle.label | password | +| SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | semmle.label | request : HttpServletRequest | +| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | semmle.label | getRequestParameter(...) | +| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) : String | semmle.label | getRequestParameter(...) : String | +| SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | semmle.label | request : HttpServletRequest | +| SensitiveGetQuery3.java:13:57:13:64 | password | semmle.label | password | +| SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | semmle.label | request : HttpServletRequest | +| SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | semmle.label | request : HttpServletRequest | | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | semmle.label | getParameter(...) | | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | semmle.label | getParameter(...) : String | | SensitiveGetQuery.java:14:29:14:36 | password | semmle.label | password | @@ -27,9 +46,16 @@ nodes | SensitiveGetQuery.java:17:40:17:54 | password : String | semmle.label | password : String | | SensitiveGetQuery.java:18:61:18:68 | password | semmle.label | password | #select -| SensitiveGetQuery2.java:14:30:14:48 | get(...) | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:14:30:14:48 | get(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | sensitive query string | -| SensitiveGetQuery2.java:15:29:15:36 | password | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:15:29:15:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | sensitive query string | -| SensitiveGetQuery2.java:19:61:19:68 | password | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) : Map | SensitiveGetQuery2.java:19:61:19:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:37 | getParameterMap(...) | sensitive query string | -| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | sensitive query string | -| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | sensitive query string | -| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) : String | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | sensitive query string | +| SensitiveGetQuery2.java:14:30:14:48 | get(...) | SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:14:30:14:48 | get(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:19 | request | sensitive query string | +| SensitiveGetQuery2.java:15:29:15:36 | password | SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:15:29:15:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:19 | request | sensitive query string | +| SensitiveGetQuery2.java:19:61:19:68 | password | SensitiveGetQuery2.java:12:13:12:19 | request : HttpServletRequest | SensitiveGetQuery2.java:19:61:19:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery2.java:12:13:12:19 | request | sensitive query string | +| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:11:41:11:47 | request | sensitive query string | +| SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:12:21:12:60 | getRequestParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:12:41:12:47 | request | sensitive query string | +| SensitiveGetQuery3.java:13:57:13:64 | password | SensitiveGetQuery3.java:11:41:11:47 | request : HttpServletRequest | SensitiveGetQuery3.java:13:57:13:64 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:11:41:11:47 | request | sensitive query string | +| SensitiveGetQuery3.java:13:57:13:64 | password | SensitiveGetQuery3.java:12:41:12:47 | request : HttpServletRequest | SensitiveGetQuery3.java:13:57:13:64 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery3.java:12:41:12:47 | request | sensitive query string | +| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:11:21:11:27 | request | sensitive query string | +| SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:12:21:12:52 | getParameter(...) | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:27 | request | sensitive query string | +| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:11:21:11:27 | request | sensitive query string | +| SensitiveGetQuery.java:14:29:14:36 | password | SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:14:29:14:36 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:27 | request | sensitive query string | +| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:11:21:11:27 | request : HttpServletRequest | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:11:21:11:27 | request | sensitive query string | +| SensitiveGetQuery.java:18:61:18:68 | password | SensitiveGetQuery.java:12:21:12:27 | request : HttpServletRequest | SensitiveGetQuery.java:18:61:18:68 | password | $@ uses GET request method with sensitive information. | SensitiveGetQuery.java:12:21:12:27 | request | sensitive query string | diff --git a/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery3.java b/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery3.java new file mode 100644 index 00000000000..3f30a17a856 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-598/SensitiveGetQuery3.java @@ -0,0 +1,26 @@ +import java.io.IOException; + +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.ServletException; + +public class SensitiveGetQuery3 extends HttpServlet { + // BAD - Tests sending sensitive information in a GET request. + public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { + String username = getRequestParameter(request, "username"); + String password = getRequestParameter(request, "password"); + System.out.println("Username="+username+"; password="+password); + } + + String getRequestParameter(HttpServletRequest request, String paramName) { + return request.getParameter(paramName); + } + + // GOOD - Tests sending sensitive information in a POST request. + public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { + String username = getRequestParameter(request, "username"); + String password = getRequestParameter(request, "password"); + System.out.println("Username="+username+"; password="+password); + } +}