diff --git a/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml b/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml new file mode 100644 index 00000000000..f1c7e90f0e8 --- /dev/null +++ b/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: sinkModel + data: + - ["org.apache.struts2.dispatcher", "SessionMap", False, "put", "", "", "Argument[0..1]", "trust-boundary", "manual"] diff --git a/java/ql/lib/ext/org.apache.struts2.interceptor.model.yml b/java/ql/lib/ext/org.apache.struts2.interceptor.model.yml new file mode 100644 index 00000000000..da6a83c2af4 --- /dev/null +++ b/java/ql/lib/ext/org.apache.struts2.interceptor.model.yml @@ -0,0 +1,7 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: sinkModel + data: + - ["org.apache.struts2.interceptor", "SessionAware", False, "setSession", "", "", "Argument[0]", "trust-boundary", "manual"] + - ["org.apache.struts2.interceptor", "SessionAware", False, "withSession", "", "", "Argument[0]", "trust-boundary", "manual"] \ No newline at end of file diff --git a/java/ql/lib/ext/play.mvc.model.yml b/java/ql/lib/ext/play.mvc.model.yml index ba9a11c3f78..85d1c3c3e8d 100644 --- a/java/ql/lib/ext/play.mvc.model.yml +++ b/java/ql/lib/ext/play.mvc.model.yml @@ -16,6 +16,11 @@ extensions: - ["play.mvc", "Http$RequestHeader", True, "queryString", "", "", "ReturnValue", "remote", "manual"] - ["play.mvc", "Http$RequestHeader", True, "remoteAddress", "", "", "ReturnValue", "remote", "manual"] - ["play.mvc", "Http$RequestHeader", True, "uri", "", "", "ReturnValue", "remote", "manual"] + - addsTo: + pack: codeql/java-all + extensible: sinkModel + data: + - ["play.mvc", "Result", False, "addingToSession", "", "", "Argument[1..2]", "trust-boundary", "manual"] - addsTo: pack: codeql/java-all extensible: summaryModel diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 5af886096e0..2037abadaac 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -7,11 +7,21 @@ private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.frameworks.Servlets class TrustBoundaryViolationSource extends DataFlow::Node { - TrustBoundaryViolationSource() { - this instanceof RemoteFlowSource and this.asExpr().getType() instanceof HttpServletRequest - } + TrustBoundaryViolationSource() { this.asExpr().getType() instanceof HttpServletRequest } } class TrustBoundaryViolationSink extends DataFlow::Node { TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") } } + +module TrustBoundaryConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof TrustBoundaryViolationSource } + + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + n2.asExpr().(MethodAccess).getQualifier() = n1.asExpr() + } + + predicate isSink(DataFlow::Node sink) { sink instanceof TrustBoundaryViolationSink } +} + +module TrustBoundaryFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql index 7a2a1610d03..698cbaab14e 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql @@ -14,58 +14,6 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.TrustBoundaryViolationQuery - -/** - * The `setAttribute` method of the `HttpSession` interface. - */ -abstract class SessionSetAttributeMethod extends Method { - abstract int getArgumentIndex(); -} - -private class PlayMvcResultAddingToSessionMethod extends SessionSetAttributeMethod { - PlayMvcResultAddingToSessionMethod() { - this.getDeclaringType().hasQualifiedName("play.mvc", "Result") and - this.hasName("addingToSession") - } - - override int getArgumentIndex() { result = [1, 2] } -} - -private class Struts2SessionMapPutMethod extends SessionSetAttributeMethod { - Struts2SessionMapPutMethod() { - this.getDeclaringType().hasQualifiedName("org.apache.struts2.dispatcher", "SessionMap") and - this.hasName("put") - } - - override int getArgumentIndex() { result = 1 } -} - -private class Struts2SessionSetMethod extends SessionSetAttributeMethod { - Struts2SessionSetMethod() { - this.getDeclaringType().hasQualifiedName("org.apache.struts2.interceptor", "SessionAware") and - this.hasName(["setSession", "withSession"]) - } - - override int getArgumentIndex() { result = 0 } -} - -module TrustBoundaryConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source instanceof RemoteFlowSource and - source.asExpr().(MethodAccess).getQualifier().getType() instanceof HttpServletRequest - } - - predicate isSink(DataFlow::Node sink) { - exists(MethodAccess ma, SessionSetAttributeMethod m | m = ma.getMethod() | - sink.asExpr() = ma.getArgument(m.getArgumentIndex()) - ) - or - sink instanceof TrustBoundaryViolationSink - } -} - -module TrustBoundaryFlow = TaintTracking::Global; - import TrustBoundaryFlow::PathGraph from TrustBoundaryFlow::PathNode source, TrustBoundaryFlow::PathNode sink