From 76438f13b640c2284a7e6d5ae3a06122883da3bb Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 5 Jun 2023 15:55:02 -0400 Subject: [PATCH 01/24] Trust Boundary Query --- java/ql/lib/ext/javax.servlet.http.model.yml | 2 + .../security/TrustBoundaryViolationQuery.qll | 17 +++++ .../CWE/CWE-501/TrustBoundaryViolation.ql | 75 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll create mode 100644 java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql diff --git a/java/ql/lib/ext/javax.servlet.http.model.yml b/java/ql/lib/ext/javax.servlet.http.model.yml index 6485ea22a2e..1735491f4eb 100644 --- a/java/ql/lib/ext/javax.servlet.http.model.yml +++ b/java/ql/lib/ext/javax.servlet.http.model.yml @@ -26,6 +26,8 @@ extensions: - ["javax.servlet.http", "HttpServletResponse", False, "addHeader", "", "", "Argument[0..1]", "response-splitting", "manual"] - ["javax.servlet.http", "HttpServletResponse", False, "sendError", "(int,String)", "", "Argument[1]", "information-leak", "manual"] - ["javax.servlet.http", "HttpServletResponse", False, "setHeader", "", "", "Argument[0..1]", "response-splitting", "manual"] + - ["javax.servlet.http", "HttpSession", True, "putValue", "", "", "Argument[0..1]", "trust-boundary", "manual"] + - ["javax.servlet.http", "HttpSession", True, "setAttribute", "", "", "Argument[0..1]", "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 new file mode 100644 index 00000000000..5af886096e0 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -0,0 +1,17 @@ +/** Provides classes and predicates to reason about trust boundary violations */ + +import java +private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.ExternalFlow +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 + } +} + +class TrustBoundaryViolationSink extends DataFlow::Node { + TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") } +} diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql new file mode 100644 index 00000000000..7a2a1610d03 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql @@ -0,0 +1,75 @@ +/** + * @id java/trust-boundary-violation + * @name Trust boundary violation + * @description A user-provided value is used to set a session attribute. + * @kind path-problem + * @problem.severity error + * @precision medium + * @tags security + * external/cwe/cwe-501 + */ + +import java +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 +where TrustBoundaryFlow::flowPath(source, sink) +select sink.getNode(), sink, source, + "This servlet reads data from a remote source and writes it to a $@.", sink.getNode(), + "session variable" From a8b7e70d01fb878fda1bad732e8f311bf7676616 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Thu, 8 Jun 2023 10:54:07 -0400 Subject: [PATCH 02/24] Convert trust boundary models to MaD --- .../org.apache.struts2.dispatcher.model.yml | 6 +++ .../org.apache.struts2.interceptor.model.yml | 7 +++ java/ql/lib/ext/play.mvc.model.yml | 5 ++ .../security/TrustBoundaryViolationQuery.qll | 16 ++++-- .../CWE/CWE-501/TrustBoundaryViolation.ql | 52 ------------------- 5 files changed, 31 insertions(+), 55 deletions(-) create mode 100644 java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml create mode 100644 java/ql/lib/ext/org.apache.struts2.interceptor.model.yml 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 From 15370506b82073db5dbe30d14a71516f0be8f957 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 9 Jun 2023 20:15:26 -0400 Subject: [PATCH 03/24] Add missing security severity --- java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql index 698cbaab14e..3e64463bcc9 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql @@ -4,6 +4,7 @@ * @description A user-provided value is used to set a session attribute. * @kind path-problem * @problem.severity error + * @security-severity 8.8 * @precision medium * @tags security * external/cwe/cwe-501 From 3e7444cd662104ddf27f50dd1484a55d3dc6389f Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 9 Jun 2023 20:18:56 -0400 Subject: [PATCH 04/24] Style fixes --- java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql index 3e64463bcc9..93b433521e6 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql @@ -11,14 +11,10 @@ */ import java -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.TrustBoundaryViolationQuery import TrustBoundaryFlow::PathGraph from TrustBoundaryFlow::PathNode source, TrustBoundaryFlow::PathNode sink where TrustBoundaryFlow::flowPath(source, sink) select sink.getNode(), sink, source, - "This servlet reads data from a remote source and writes it to a $@.", sink.getNode(), - "session variable" + "This servlet reads data from a remote source and writes it to a session variable." From b9f2da7875403f387b2c85736b55463cb53c6e9e Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Thu, 15 Jun 2023 10:04:59 -0400 Subject: [PATCH 05/24] Comments and import fixes --- .../security/TrustBoundaryViolationQuery.qll | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 2037abadaac..39e0fb3fef1 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -4,16 +4,29 @@ import java private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources -private import semmle.code.java.frameworks.Servlets -class TrustBoundaryViolationSource extends DataFlow::Node { - TrustBoundaryViolationSource() { this.asExpr().getType() instanceof HttpServletRequest } +/** + * A source of data that crosses a trust boundary. + */ +abstract class TrustBoundaryViolationSource extends DataFlow::Node { } + +/** + * A node representing a servlet request. + */ +private class ServletRequestSource extends TrustBoundaryViolationSource { + ServletRequestSource() { this.asExpr().getType() instanceof HttpServletRequest } } +/** + * A sink for data that crosses a trust boundary. + */ class TrustBoundaryViolationSink extends DataFlow::Node { TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") } } +/** + * Taint tracking for data that crosses a trust boundary. + */ module TrustBoundaryConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof TrustBoundaryViolationSource } @@ -24,4 +37,7 @@ module TrustBoundaryConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof TrustBoundaryViolationSink } } +/** + * Taint-tracking flow for values which cross a trust boundary. + */ module TrustBoundaryFlow = TaintTracking::Global; From ab9f0240d3ff6318a2b68c553c3708de529acdbd Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 10 Jul 2023 01:43:12 -0400 Subject: [PATCH 06/24] Add taint steps for HTML encoding methods --- java/ql/lib/ext/org.apache.commons.lang.model.yml | 7 +++++++ java/ql/lib/ext/org.owasp.esapi.model.yml | 6 ++++++ 2 files changed, 13 insertions(+) create mode 100644 java/ql/lib/ext/org.apache.commons.lang.model.yml create mode 100644 java/ql/lib/ext/org.owasp.esapi.model.yml diff --git a/java/ql/lib/ext/org.apache.commons.lang.model.yml b/java/ql/lib/ext/org.apache.commons.lang.model.yml new file mode 100644 index 00000000000..8dd3fd003f9 --- /dev/null +++ b/java/ql/lib/ext/org.apache.commons.lang.model.yml @@ -0,0 +1,7 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: summaryModel + data: + - ["org.apache.commons.lang", "StringEscapeUtils", true, "escapeHtml", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"] + - ["org.apache.commons.lang", "StringEscapeUtils", true, "escapeHtml", "(Writer,String)", "", "Argument[1]", "Argument[0]", "taint", "manual"] diff --git a/java/ql/lib/ext/org.owasp.esapi.model.yml b/java/ql/lib/ext/org.owasp.esapi.model.yml new file mode 100644 index 00000000000..30578debe58 --- /dev/null +++ b/java/ql/lib/ext/org.owasp.esapi.model.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: summaryModel + data: + - ["org.owasp.esapi", "Encoder", true, "encodeForHTML", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"] \ No newline at end of file From 2aba425464ee8d6cff6cb55313a356837f53409e Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 10 Jul 2023 01:58:20 -0400 Subject: [PATCH 07/24] TrustBoundary test ql file --- .../security/CWE-501/TrustBoundaryViolations.expected | 2 ++ .../query-tests/security/CWE-501/TrustBoundaryViolations.ql | 4 ++++ 2 files changed, 6 insertions(+) create mode 100644 java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.expected create mode 100644 java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.ql diff --git a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.expected b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.expected new file mode 100644 index 00000000000..48de9172b36 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.expected @@ -0,0 +1,2 @@ +failures +testFailures diff --git a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.ql b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.ql new file mode 100644 index 00000000000..26a9b4a7308 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.ql @@ -0,0 +1,4 @@ +import java +import semmle.code.java.security.TrustBoundaryViolationQuery +import TestUtilities.InlineFlowTest +import TaintFlowTest From f58590c6a983e7d3e96ce780b5785661ee336f61 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Wed, 19 Jul 2023 16:39:09 -0400 Subject: [PATCH 08/24] Trust Boundary Work --- .../semmle/code/java/frameworks/Servlets.qll | 4 ++ .../security/TrustBoundaryViolationQuery.qll | 11 ++++++ .../CWE/CWE-501/TrustBoundaryViolation.qhelp | 39 +++++++++++++++++++ .../CWE-501/TrustBoundaryViolations.java | 12 ++++++ .../test/query-tests/security/CWE-501/options | 0 5 files changed, 66 insertions(+) create mode 100644 java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp create mode 100644 java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java create mode 100644 java/ql/test/query-tests/security/CWE-501/options diff --git a/java/ql/lib/semmle/code/java/frameworks/Servlets.qll b/java/ql/lib/semmle/code/java/frameworks/Servlets.qll index f2de51b2aab..bc080fcb48f 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Servlets.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Servlets.qll @@ -397,3 +397,7 @@ class GetServletResourceAsStreamMethod extends Method { this.hasName("getResourceAsStream") } } + +class HttpServletSession extends RefType { + HttpServletSession() { this.hasQualifiedName("javax.servlet.http", "HttpSession") } +} diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 39e0fb3fef1..60801fceff6 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -24,6 +24,8 @@ class TrustBoundaryViolationSink extends DataFlow::Node { TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") } } +abstract class TrustBoundaryValidationSanitizer extends DataFlow::Node { } + /** * Taint tracking for data that crosses a trust boundary. */ @@ -34,6 +36,15 @@ module TrustBoundaryConfig implements DataFlow::ConfigSig { n2.asExpr().(MethodAccess).getQualifier() = n1.asExpr() } + predicate isBarrier(DataFlow::Node node) { + node instanceof TrustBoundaryValidationSanitizer or + node.getType() instanceof HttpServletSession or + node.asExpr() + .(MethodAccess) + .getMethod() + .hasQualifiedName("javax.servlet.http", "HttpServletRequest", "getMethod") + } + predicate isSink(DataFlow::Node sink) { sink instanceof TrustBoundaryViolationSink } } diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp new file mode 100644 index 00000000000..2c6148129d3 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp @@ -0,0 +1,39 @@ + + + +

+ A trust boundary violation occurs when a value is passed from a less trusted context to a more trusted context. +

+ +

+ For example, a value that is generated by a less trusted source, such as a user, may be passed to a more trusted + source, such as a system process. If the less trusted source is malicious, then the value may be crafted to + exploit the more trusted source. +

+ +

+ Trust boundary violations are often caused by a failure to validate input. For example, if a web application + accepts a cookie from a user, then the application should validate the cookie before using it. If the cookie is + not validated, then the user may be able to craft a malicious cookie that exploits the application. +

+ + + +

+ Validate input coming from a user. For example, if a web application accepts a cookie from a user, then the + application should validate the cookie before using it. +

+ + + + + + +
  • + Wikipedia: Trust boundary. +
    • +     + +     diff --git a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java new file mode 100644 index 00000000000..5bef7e087d2 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java @@ -0,0 +1,12 @@ +import java.io.IOException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class TrustBoundaryViolations extends HttpServlet { + public void doGet(HttpServletRequest request, HttpServletResponse response) { + String input = request.getParameter("input"); + + request.getSession().setAttribute("input", input); // $ hasTaintFlow + } +} diff --git a/java/ql/test/query-tests/security/CWE-501/options b/java/ql/test/query-tests/security/CWE-501/options new file mode 100644 index 00000000000..e69de29bb2d From 97d6e82869a95cb4eaf66480899aa4a2b5032a02 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 25 Jul 2023 11:57:08 -0400 Subject: [PATCH 09/24] Stubs for `org.owasp.esapi` --- .../test/query-tests/security/CWE-501/options | 1 + .../org/owasp/esapi/ValidationErrorList.java | 5 + .../org/owasp/esapi/Validator.java | 224 ++++++++++++++++++ .../esapi/errors/IntrusionException.java | 5 + .../esapi/errors/ValidationException.java | 5 + 5 files changed, 240 insertions(+) create mode 100644 java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/ValidationErrorList.java create mode 100644 java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/Validator.java create mode 100644 java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/IntrusionException.java create mode 100644 java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/ValidationException.java diff --git a/java/ql/test/query-tests/security/CWE-501/options b/java/ql/test/query-tests/security/CWE-501/options index e69de29bb2d..37d627da7e8 100644 --- a/java/ql/test/query-tests/security/CWE-501/options +++ b/java/ql/test/query-tests/security/CWE-501/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/esapi-2.0.1:${testdir}/../../../stubs/javax-servlet-2.5 diff --git a/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/ValidationErrorList.java b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/ValidationErrorList.java new file mode 100644 index 00000000000..ae56481a974 --- /dev/null +++ b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/ValidationErrorList.java @@ -0,0 +1,5 @@ +package org.owasp.esapi; + +public class ValidationErrorList { + +} diff --git a/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/Validator.java b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/Validator.java new file mode 100644 index 00000000000..8bee29c1c38 --- /dev/null +++ b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/Validator.java @@ -0,0 +1,224 @@ +package org.owasp.esapi; + +import java.io.File; +import java.io.InputStream; +import java.net.URI; +import java.text.DateFormat; +import java.util.Date; +import java.util.List; +import java.util.Set; + +import javax.servlet.http.HttpServletRequest; + +import org.owasp.esapi.errors.IntrusionException; +import org.owasp.esapi.errors.ValidationException; + +public interface Validator { + + boolean isValidInput(String context, String input, String type, int maxLength, boolean allowNull) + throws IntrusionException; + + boolean isValidInput(String context, String input, String type, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidInput(String context, String input, String type, int maxLength, boolean allowNull, + boolean canonicalize) throws IntrusionException; + + boolean isValidInput(String context, String input, String type, int maxLength, boolean allowNull, + boolean canonicalize, ValidationErrorList errorList) throws IntrusionException; + + String getValidInput(String context, String input, String type, int maxLength, boolean allowNull) + throws ValidationException, IntrusionException; + + String getValidInput(String context, String input, String type, int maxLength, boolean allowNull, + boolean canonicalize) throws ValidationException, IntrusionException; + + String getValidInput(String context, String input, String type, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + String getValidInput(String context, String input, String type, int maxLength, boolean allowNull, + boolean canonicalize, ValidationErrorList errorList) throws IntrusionException; + + boolean isValidDate(String context, String input, DateFormat format, boolean allowNull) throws IntrusionException; + + boolean isValidDate(String context, String input, DateFormat format, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + Date getValidDate(String context, String input, DateFormat format, boolean allowNull) + throws ValidationException, IntrusionException; + + Date getValidDate(String context, String input, DateFormat format, boolean allowNull, ValidationErrorList errorList) + throws IntrusionException; + + boolean isValidSafeHTML(String context, String input, int maxLength, boolean allowNull) throws IntrusionException; + + boolean isValidSafeHTML(String context, String input, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + String getValidSafeHTML(String context, String input, int maxLength, boolean allowNull) + throws ValidationException, IntrusionException; + + String getValidSafeHTML(String context, String input, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidCreditCard(String context, String input, boolean allowNull) throws IntrusionException; + + boolean isValidCreditCard(String context, String input, boolean allowNull, ValidationErrorList errorList) + throws IntrusionException; + + String getValidCreditCard(String context, String input, boolean allowNull) + throws ValidationException, IntrusionException; + + String getValidCreditCard(String context, String input, boolean allowNull, ValidationErrorList errorList) + throws IntrusionException; + + boolean isValidDirectoryPath(String context, String input, File parent, boolean allowNull) + throws IntrusionException; + + boolean isValidDirectoryPath(String context, String input, File parent, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + String getValidDirectoryPath(String context, String input, File parent, boolean allowNull) + throws ValidationException, IntrusionException; + + String getValidDirectoryPath(String context, String input, File parent, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidFileName(String context, String input, boolean allowNull) throws IntrusionException; + + boolean isValidFileName(String context, String input, boolean allowNull, ValidationErrorList errorList) + throws IntrusionException; + + boolean isValidFileName(String context, String input, List allowedExtensions, boolean allowNull) + throws IntrusionException; + + boolean isValidFileName(String context, String input, List allowedExtensions, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + String getValidFileName(String context, String input, List allowedExtensions, boolean allowNull) + throws ValidationException, IntrusionException; + + String getValidFileName(String context, String input, List allowedExtensions, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidNumber(String context, String input, long minValue, long maxValue, boolean allowNull) + throws IntrusionException; + + boolean isValidNumber(String context, String input, long minValue, long maxValue, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + Double getValidNumber(String context, String input, long minValue, long maxValue, boolean allowNull) + throws ValidationException, IntrusionException; + + Double getValidNumber(String context, String input, long minValue, long maxValue, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidInteger(String context, String input, int minValue, int maxValue, boolean allowNull) + throws IntrusionException; + + boolean isValidInteger(String context, String input, int minValue, int maxValue, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + Integer getValidInteger(String context, String input, int minValue, int maxValue, boolean allowNull) + throws ValidationException, IntrusionException; + + Integer getValidInteger(String context, String input, int minValue, int maxValue, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidDouble(String context, String input, double minValue, double maxValue, boolean allowNull) + throws IntrusionException; + + boolean isValidDouble(String context, String input, double minValue, double maxValue, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + Double getValidDouble(String context, String input, double minValue, double maxValue, boolean allowNull) + throws ValidationException, IntrusionException; + + Double getValidDouble(String context, String input, double minValue, double maxValue, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidFileContent(String context, byte[] input, int maxBytes, boolean allowNull) throws IntrusionException; + + boolean isValidFileContent(String context, byte[] input, int maxBytes, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + byte[] getValidFileContent(String context, byte[] input, int maxBytes, boolean allowNull) + throws ValidationException, IntrusionException; + + byte[] getValidFileContent(String context, byte[] input, int maxBytes, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidFileUpload(String context, String filepath, String filename, File parent, byte[] content, + int maxBytes, boolean allowNull) throws IntrusionException; + + boolean isValidFileUpload(String context, String filepath, String filename, File parent, byte[] content, + int maxBytes, boolean allowNull, ValidationErrorList errorList) throws IntrusionException; + + void assertValidFileUpload(String context, String filepath, String filename, File parent, byte[] content, + int maxBytes, List allowedExtensions, boolean allowNull) + throws ValidationException, IntrusionException; + + void assertValidFileUpload(String context, String filepath, String filename, File parent, byte[] content, + int maxBytes, List allowedExtensions, boolean allowNull, ValidationErrorList errorList) + throws IntrusionException; + + boolean isValidListItem(String context, String input, List list) throws IntrusionException; + + boolean isValidListItem(String context, String input, List list, ValidationErrorList errorList) + throws IntrusionException; + + String getValidListItem(String context, String input, List list) + throws ValidationException, IntrusionException; + + String getValidListItem(String context, String input, List list, ValidationErrorList errorList) + throws IntrusionException; + + boolean isValidHTTPRequestParameterSet(String context, HttpServletRequest request, Set required, + Set optional) throws IntrusionException; + + boolean isValidHTTPRequestParameterSet(String context, HttpServletRequest request, Set required, + Set optional, ValidationErrorList errorList) throws IntrusionException; + + void assertValidHTTPRequestParameterSet(String context, HttpServletRequest request, Set required, + Set optional) throws ValidationException, IntrusionException; + + void assertValidHTTPRequestParameterSet(String context, HttpServletRequest request, Set required, + Set optional, ValidationErrorList errorList) throws IntrusionException; + + boolean isValidPrintable(String context, char[] input, int maxLength, boolean allowNull) throws IntrusionException; + + boolean isValidPrintable(String context, char[] input, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + char[] getValidPrintable(String context, char[] input, int maxLength, boolean allowNull) throws ValidationException; + + char[] getValidPrintable(String context, char[] input, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidPrintable(String context, String input, int maxLength, boolean allowNull) throws IntrusionException; + + boolean isValidPrintable(String context, String input, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + String getValidPrintable(String context, String input, int maxLength, boolean allowNull) throws ValidationException; + + String getValidPrintable(String context, String input, int maxLength, boolean allowNull, + ValidationErrorList errorList) throws IntrusionException; + + boolean isValidRedirectLocation(String context, String input, boolean allowNull); + + boolean isValidRedirectLocation(String context, String input, boolean allowNull, ValidationErrorList errorList); + + String getValidRedirectLocation(String context, String input, boolean allowNull) + throws ValidationException, IntrusionException; + + String getValidRedirectLocation(String context, String input, boolean allowNull, ValidationErrorList errorList) + throws IntrusionException; + + String safeReadLine(InputStream inputStream, int maxLength) throws ValidationException; + + boolean isValidURI(String context, String input, boolean allowNull); + + URI getRfcCompliantURI(String input); + +} \ No newline at end of file diff --git a/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/IntrusionException.java b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/IntrusionException.java new file mode 100644 index 00000000000..2ecbcd6c449 --- /dev/null +++ b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/IntrusionException.java @@ -0,0 +1,5 @@ +package org.owasp.esapi.errors; + +public class IntrusionException extends Exception { + +} diff --git a/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/ValidationException.java b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/ValidationException.java new file mode 100644 index 00000000000..d8158b984f9 --- /dev/null +++ b/java/ql/test/stubs/esapi-2.0.1/org/owasp/esapi/errors/ValidationException.java @@ -0,0 +1,5 @@ +package org.owasp.esapi.errors; + +public class ValidationException extends Exception { + +} From 55fae2daaa9420bbdb4f29f172c44aa6283987c1 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 25 Jul 2023 11:58:36 -0400 Subject: [PATCH 10/24] Added ESAPI sanitizer --- .../code/java/frameworks/owasp/Esapi.qll | 40 +++++++++++++++++++ .../security/TrustBoundaryViolationQuery.qll | 23 +++++++++++ .../CWE-501/TrustBoundaryViolations.java | 23 +++++++++++ 3 files changed, 86 insertions(+) create mode 100644 java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll diff --git a/java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll b/java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll new file mode 100644 index 00000000000..19cabda7073 --- /dev/null +++ b/java/ql/lib/semmle/code/java/frameworks/owasp/Esapi.qll @@ -0,0 +1,40 @@ +/** Classes and predicates for reasoning about the `owasp.easpi` package. */ + +import java + +/** + * The `org.owasp.esapi.Validator` interface. + */ +class EsapiValidator extends RefType { + EsapiValidator() { this.hasQualifiedName("org.owasp.esapi", "Validator") } +} + +/** + * The methods of `org.owasp.esapi.Validator` which validate data. + */ +class EsapiIsValidMethod extends Method { + EsapiIsValidMethod() { + this.getDeclaringType() instanceof EsapiValidator and + this.hasName([ + "isValidCreditCard", "isValidDate", "isValidDirectoryPath", "isValidDouble", + "isValidFileContent", "isValidFileName", "isValidInput", "isValidInteger", + "isValidListItem", "isValidNumber", "isValidPrintable", "isValidRedirectLocation", + "isValidSafeHTML", "isValidURI" + ]) + } +} + +/** + * The methods of `org.owasp.esapi.Validator` which return validated data. + */ +class EsapiGetValidMethod extends Method { + EsapiGetValidMethod() { + this.getDeclaringType() instanceof EsapiValidator and + this.hasName([ + "getValidCreditCard", "getValidDate", "getValidDirectoryPath", "getValidDouble", + "getValidFileContent", "getValidFileName", "getValidInput", "getValidInteger", + "getValidListItem", "getValidNumber", "getValidPrintable", "getValidRedirectLocation", + "getValidSafeHTML", "getValidURI" + ]) + } +} diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 60801fceff6..1f84f98018f 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -2,8 +2,10 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.controlflow.Guards private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources +private import semmle.code.java.frameworks.owasp.Esapi /** * A source of data that crosses a trust boundary. @@ -26,6 +28,27 @@ class TrustBoundaryViolationSink extends DataFlow::Node { abstract class TrustBoundaryValidationSanitizer extends DataFlow::Node { } +/** + * A node validated by an OWASP ESAPI validation method. + */ +private class EsapiValidatedInputSanitizer extends TrustBoundaryValidationSanitizer { + EsapiValidatedInputSanitizer() { + this = DataFlow::BarrierGuard::getABarrierNode() or + this.asExpr().(MethodAccess).getMethod() instanceof EsapiGetValidMethod + } +} + +/** + * Holds if `g` is a guard that checks that `e` is valid data according to an OWASP ESAPI validation method. + */ +private predicate esapiIsValidData(Guard g, Expr e, boolean branch) { + branch = true and + exists(MethodAccess ma | ma.getMethod() instanceof EsapiIsValidMethod | + g = ma and + e = ma.getArgument(1) + ) +} + /** * Taint tracking for data that crosses a trust boundary. */ diff --git a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java index 5bef7e087d2..9f9be44c972 100644 --- a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java +++ b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java @@ -2,11 +2,34 @@ import java.io.IOException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.owasp.esapi.Validator; public class TrustBoundaryViolations extends HttpServlet { + Validator validator; + public void doGet(HttpServletRequest request, HttpServletResponse response) { String input = request.getParameter("input"); + // BAD: The input is written to the response without being sanitized. request.getSession().setAttribute("input", input); // $ hasTaintFlow + + String input2 = request.getParameter("input2"); + + try { + String sanitized = validator.getValidInput("HTTP parameter", input2, "HTTPParameterValue", 100, false); + // GOOD: The input is sanitized before being written to the response. + request.getSession().setAttribute("input2", sanitized); + + } catch (Exception e) { + } + + try { + String input3 = request.getParameter("input3"); + if (validator.isValidInput("HTTP parameter", input3, "HTTPParameterValue", 100, false)) { + // GOOD: The input is sanitized before being written to the response. + request.getSession().setAttribute("input3", input3); + } + } catch (Exception e) { + } } } From b567ec875a640a5dd0e7df09ce873491fbb6760e Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 25 Jul 2023 21:05:54 -0400 Subject: [PATCH 11/24] Documentation --- .../java/security/TrustBoundaryViolationQuery.qll | 3 +++ .../Security/CWE/CWE-501/TrustBoundaryFixed.java | 8 ++++++++ .../CWE/CWE-501/TrustBoundaryViolation.qhelp | 13 +++++++++++-- .../CWE/CWE-501/TrustBoundaryVulnerable.java | 6 ++++++ 4 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java create mode 100644 java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 1f84f98018f..337c228bc75 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -26,6 +26,9 @@ class TrustBoundaryViolationSink extends DataFlow::Node { TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") } } +/** + * A sanitizer for data that crosses a trust boundary. + */ abstract class TrustBoundaryValidationSanitizer extends DataFlow::Node { } /** diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java new file mode 100644 index 00000000000..d9d3a29f314 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java @@ -0,0 +1,8 @@ +public void doGet(HttpServletRequest request, HttpServletResponse response) { + String username = request.getParameter("username"); + + if (validator.isValidInput("HTTP parameter", username, "username", 20, false)) { + // GOOD: The input is sanitized before being written to the response. + request.getSession().setAttribute("username", username); + } +} \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp index 2c6148129d3..d4a5af8ed38 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp @@ -22,12 +22,21 @@

    - Validate input coming from a user. For example, if a web application accepts a cookie from a user, then the - application should validate the cookie before using it. + In order to maintain a trust boundary, data from less trusted sources should be validated before being used.

    +

    + In the first (bad) example, the server accepts a parameter from the user and uses it to set the username without validation. +

    + + +

    + In the second (good) example, the server validates the parameter before using it to set the username. +

    + +     diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java new file mode 100644 index 00000000000..f3a38f8e22f --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java @@ -0,0 +1,6 @@ +public void doGet(HttpServletRequest request, HttpServletResponse response) { + String username = request.getParameter("username"); + + // BAD: The input is written to the response without being sanitized. + request.getSession().setAttribute("username", username); +} \ No newline at end of file From 172b8a6967d1371b71dfd9017b1e8a3792284c01 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 25 Jul 2023 21:11:49 -0400 Subject: [PATCH 12/24] Documentation fixes --- java/ql/lib/semmle/code/java/frameworks/Servlets.qll | 1 + java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/java/ql/lib/semmle/code/java/frameworks/Servlets.qll b/java/ql/lib/semmle/code/java/frameworks/Servlets.qll index bc080fcb48f..9c195ecea8d 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Servlets.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Servlets.qll @@ -398,6 +398,7 @@ class GetServletResourceAsStreamMethod extends Method { } } +/** The interface `javax.servlet.http.HttpSession` */ class HttpServletSession extends RefType { HttpServletSession() { this.hasQualifiedName("javax.servlet.http", "HttpSession") } } diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp index d4a5af8ed38..e3491e9bcf8 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp @@ -30,12 +30,12 @@

    In the first (bad) example, the server accepts a parameter from the user and uses it to set the username without validation.

    - +

    In the second (good) example, the server validates the parameter before using it to set the username.

    - + From 52ebf9fff6b00c4bdc44bb4d1b6fb2a24134e397 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 25 Jul 2023 21:21:18 -0400 Subject: [PATCH 13/24] Java: Add trust boundary change note --- .../2023-07-25-trust-boundary-violation-query.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md diff --git a/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md b/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md new file mode 100644 index 00000000000..42ee360ec1a --- /dev/null +++ b/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md @@ -0,0 +1,5 @@ +--- +category: newQuery +--- +* Added the `java/trust-boundary-violation` query to detect trust boundary violations between http requests and the http session. + From 929090a8470dc790a455f068d9bd9fbca62e0126 Mon Sep 17 00:00:00 2001 From: Edward Minnix III Date: Wed, 26 Jul 2023 09:58:56 -0400 Subject: [PATCH 14/24] Typos and style fixes Co-authored-by: Tony Torralba --- java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java | 2 +- .../src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java | 2 +- .../2023-07-25-trust-boundary-violation-query.md | 2 +- .../security/CWE-501/TrustBoundaryViolations.java | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java index d9d3a29f314..50f14c0bc4f 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryFixed.java @@ -2,7 +2,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) { String username = request.getParameter("username"); if (validator.isValidInput("HTTP parameter", username, "username", 20, false)) { - // GOOD: The input is sanitized before being written to the response. + // GOOD: The input is sanitized before being written to the session. request.getSession().setAttribute("username", username); } } \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java index f3a38f8e22f..c6174b7113e 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryVulnerable.java @@ -1,6 +1,6 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) { String username = request.getParameter("username"); - // BAD: The input is written to the response without being sanitized. + // BAD: The input is written to the session without being sanitized. request.getSession().setAttribute("username", username); } \ No newline at end of file diff --git a/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md b/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md index 42ee360ec1a..df2e8aecf79 100644 --- a/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md +++ b/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md @@ -1,5 +1,5 @@ --- category: newQuery --- -* Added the `java/trust-boundary-violation` query to detect trust boundary violations between http requests and the http session. +* Added the `java/trust-boundary-violation` query to detect trust boundary violations between HTTP requests and the HTTP session. diff --git a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java index 9f9be44c972..dc45f7e6604 100644 --- a/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java +++ b/java/ql/test/query-tests/security/CWE-501/TrustBoundaryViolations.java @@ -10,14 +10,14 @@ public class TrustBoundaryViolations extends HttpServlet { public void doGet(HttpServletRequest request, HttpServletResponse response) { String input = request.getParameter("input"); - // BAD: The input is written to the response without being sanitized. + // BAD: The input is written to the session without being sanitized. request.getSession().setAttribute("input", input); // $ hasTaintFlow String input2 = request.getParameter("input2"); try { String sanitized = validator.getValidInput("HTTP parameter", input2, "HTTPParameterValue", 100, false); - // GOOD: The input is sanitized before being written to the response. + // GOOD: The input is sanitized before being written to the session. request.getSession().setAttribute("input2", sanitized); } catch (Exception e) { @@ -26,7 +26,7 @@ public class TrustBoundaryViolations extends HttpServlet { try { String input3 = request.getParameter("input3"); if (validator.isValidInput("HTTP parameter", input3, "HTTPParameterValue", 100, false)) { - // GOOD: The input is sanitized before being written to the response. + // GOOD: The input is sanitized before being written to the session. request.getSession().setAttribute("input3", input3); } } catch (Exception e) { From a3a4c31911b561bcb9c2ae6ae98e32fc9e0f4135 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 28 Jul 2023 15:48:35 -0400 Subject: [PATCH 15/24] Replace servlet source node with RemoteFlowSource --- .../code/java/security/TrustBoundaryViolationQuery.qll | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 337c228bc75..d2a1c4c54de 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -12,11 +12,8 @@ private import semmle.code.java.frameworks.owasp.Esapi */ abstract class TrustBoundaryViolationSource extends DataFlow::Node { } -/** - * A node representing a servlet request. - */ -private class ServletRequestSource extends TrustBoundaryViolationSource { - ServletRequestSource() { this.asExpr().getType() instanceof HttpServletRequest } +private class RemoteSource extends TrustBoundaryViolationSource { + RemoteSource() { this instanceof RemoteFlowSource } } /** From e22a67e7fe2ad02f3a7007fa4e4f8e2549791e3c Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 28 Jul 2023 15:49:33 -0400 Subject: [PATCH 16/24] Remove unnecessary methods --- .../code/java/security/TrustBoundaryViolationQuery.qll | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index d2a1c4c54de..34fec734ce4 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -55,17 +55,9 @@ private predicate esapiIsValidData(Guard g, Expr e, boolean branch) { 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 isBarrier(DataFlow::Node node) { node instanceof TrustBoundaryValidationSanitizer or - node.getType() instanceof HttpServletSession or - node.asExpr() - .(MethodAccess) - .getMethod() - .hasQualifiedName("javax.servlet.http", "HttpServletRequest", "getMethod") + node.getType() instanceof HttpServletSession } predicate isSink(DataFlow::Node sink) { sink instanceof TrustBoundaryViolationSink } From 60642c52aa3e145f4d1a3bc8bedb91f42b69e35c Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 28 Jul 2023 15:59:11 -0400 Subject: [PATCH 17/24] Use non-extending subtype --- .../semmle/code/java/security/TrustBoundaryViolationQuery.qll | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 34fec734ce4..a8265069d30 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -12,9 +12,7 @@ private import semmle.code.java.frameworks.owasp.Esapi */ abstract class TrustBoundaryViolationSource extends DataFlow::Node { } -private class RemoteSource extends TrustBoundaryViolationSource { - RemoteSource() { this instanceof RemoteFlowSource } -} +private class RemoteSource extends TrustBoundaryViolationSource instanceof RemoteFlowSource { } /** * A sink for data that crosses a trust boundary. From a36c12ff1f73a1a3e14176c4ce1444c801b6c834 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 28 Jul 2023 16:54:56 -0400 Subject: [PATCH 18/24] Add trust-boundary-violation sink kind --- java/ql/lib/ext/javax.servlet.http.model.yml | 4 ++-- java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml | 2 +- java/ql/lib/ext/org.apache.struts2.interceptor.model.yml | 4 ++-- java/ql/lib/ext/play.mvc.model.yml | 2 +- .../semmle/code/java/security/TrustBoundaryViolationQuery.qll | 2 +- shared/mad/codeql/mad/ModelValidation.qll | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/java/ql/lib/ext/javax.servlet.http.model.yml b/java/ql/lib/ext/javax.servlet.http.model.yml index 1735491f4eb..c2d76a2ea7a 100644 --- a/java/ql/lib/ext/javax.servlet.http.model.yml +++ b/java/ql/lib/ext/javax.servlet.http.model.yml @@ -26,8 +26,8 @@ extensions: - ["javax.servlet.http", "HttpServletResponse", False, "addHeader", "", "", "Argument[0..1]", "response-splitting", "manual"] - ["javax.servlet.http", "HttpServletResponse", False, "sendError", "(int,String)", "", "Argument[1]", "information-leak", "manual"] - ["javax.servlet.http", "HttpServletResponse", False, "setHeader", "", "", "Argument[0..1]", "response-splitting", "manual"] - - ["javax.servlet.http", "HttpSession", True, "putValue", "", "", "Argument[0..1]", "trust-boundary", "manual"] - - ["javax.servlet.http", "HttpSession", True, "setAttribute", "", "", "Argument[0..1]", "trust-boundary", "manual"] + - ["javax.servlet.http", "HttpSession", True, "putValue", "", "", "Argument[0..1]", "trust-boundary-violation", "manual"] + - ["javax.servlet.http", "HttpSession", True, "setAttribute", "", "", "Argument[0..1]", "trust-boundary-violation", "manual"] - addsTo: pack: codeql/java-all extensible: summaryModel diff --git a/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml b/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml index f1c7e90f0e8..c15ad1cb315 100644 --- a/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml +++ b/java/ql/lib/ext/org.apache.struts2.dispatcher.model.yml @@ -3,4 +3,4 @@ extensions: pack: codeql/java-all extensible: sinkModel data: - - ["org.apache.struts2.dispatcher", "SessionMap", False, "put", "", "", "Argument[0..1]", "trust-boundary", "manual"] + - ["org.apache.struts2.dispatcher", "SessionMap", False, "put", "", "", "Argument[0..1]", "trust-boundary-violation", "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 index da6a83c2af4..4b9ccb2e093 100644 --- a/java/ql/lib/ext/org.apache.struts2.interceptor.model.yml +++ b/java/ql/lib/ext/org.apache.struts2.interceptor.model.yml @@ -3,5 +3,5 @@ extensions: 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 + - ["org.apache.struts2.interceptor", "SessionAware", False, "setSession", "", "", "Argument[0]", "trust-boundary-violation", "manual"] + - ["org.apache.struts2.interceptor", "SessionAware", False, "withSession", "", "", "Argument[0]", "trust-boundary-violation", "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 85d1c3c3e8d..3a11ddd649d 100644 --- a/java/ql/lib/ext/play.mvc.model.yml +++ b/java/ql/lib/ext/play.mvc.model.yml @@ -20,7 +20,7 @@ extensions: pack: codeql/java-all extensible: sinkModel data: - - ["play.mvc", "Result", False, "addingToSession", "", "", "Argument[1..2]", "trust-boundary", "manual"] + - ["play.mvc", "Result", False, "addingToSession", "", "", "Argument[1..2]", "trust-boundary-violation", "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 a8265069d30..52790f5e186 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -18,7 +18,7 @@ private class RemoteSource extends TrustBoundaryViolationSource instanceof Remot * A sink for data that crosses a trust boundary. */ class TrustBoundaryViolationSink extends DataFlow::Node { - TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary") } + TrustBoundaryViolationSink() { sinkNode(this, "trust-boundary-violation") } } /** diff --git a/shared/mad/codeql/mad/ModelValidation.qll b/shared/mad/codeql/mad/ModelValidation.qll index d5108c2eeec..7225c2bc1ee 100644 --- a/shared/mad/codeql/mad/ModelValidation.qll +++ b/shared/mad/codeql/mad/ModelValidation.qll @@ -33,7 +33,7 @@ module KindValidation { "bean-validation", "fragment-injection", "groovy-injection", "hostname-verification", "information-leak", "intent-redirection", "jexl-injection", "jndi-injection", "mvel-injection", "ognl-injection", "pending-intents", "response-splitting", - "template-injection", "xpath-injection", "xslt-injection", + "trust-boundary-violation", "template-injection", "xpath-injection", "xslt-injection", // JavaScript-only currently, but may be shared in the future "mongodb.sink", "nosql-injection", "unsafe-deserialization", // Swift-only currently, but may be shared in the future From b305962c9abff2a66cf18a43d02a4d0cc0ae5d1d Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 28 Jul 2023 17:05:25 -0400 Subject: [PATCH 19/24] Use more appropriate description --- java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql index 93b433521e6..9bc90f49c1f 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.ql @@ -1,7 +1,7 @@ /** * @id java/trust-boundary-violation * @name Trust boundary violation - * @description A user-provided value is used to set a session attribute. + * @description Modifying the HTTP session attributes based on data from an untrusted source may violate a trust boundary. * @kind path-problem * @problem.severity error * @security-severity 8.8 From d468ea9e9018e486905c73167a37fdfa7b155e94 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 31 Jul 2023 00:13:34 -0400 Subject: [PATCH 20/24] Add default sanitizers --- .../code/java/security/TrustBoundaryViolationQuery.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll index 52790f5e186..a89f24e6f1f 100644 --- a/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TrustBoundaryViolationQuery.qll @@ -55,7 +55,10 @@ module TrustBoundaryConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof TrustBoundaryValidationSanitizer or - node.getType() instanceof HttpServletSession + node.getType() instanceof HttpServletSession or + node.getType() instanceof NumberType or + node.getType() instanceof PrimitiveType or + node.getType() instanceof BoxedType } predicate isSink(DataFlow::Node sink) { sink instanceof TrustBoundaryViolationSink } From f53496b2a7de8004aab34440a515324db3744a90 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 1 Aug 2023 14:14:07 -0400 Subject: [PATCH 21/24] Added documentation for trust-boundary-violation sink --- .../customizing-library-models-for-java.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst b/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst index 37e1fb35450..05f7f9958d2 100644 --- a/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst +++ b/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst @@ -349,6 +349,7 @@ The following sink kinds are supported: - **response-splitting**: A sink that can be used for HTTP response splitting, such as in calls to **HttpServletResponse.setHeader**. - **sql-injection**: A sink that can be used for SQL injection, such as in a **Statement.executeQuery** call. - **template-injection**: A sink that can be used for server side template injection, such as in a **Velocity.evaluate** call. +- **trust-boundary-violation**: A sink that can be used to cross a trust boundary, such as a server's HTTP Session. - **url-redirection**: A sink that can be used to redirect the user to a malicious URL, such as in a **Response.temporaryRedirect** call. - **xpath-injection**: A sink that can be used for XPath injection, such as in a **XPath.evaluate** call. - **xslt-injection**: A sink that can be used for XSLT injection, such as in a **Transformer.transform** call. From 655a98452ac8d1d471d3e2645ef4d39491bb8879 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 1 Aug 2023 14:25:53 -0400 Subject: [PATCH 22/24] Remove `escapeHTML` models --- java/ql/lib/ext/generated/org.apache.commons.lang.model.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/java/ql/lib/ext/generated/org.apache.commons.lang.model.yml b/java/ql/lib/ext/generated/org.apache.commons.lang.model.yml index 56f9c251388..d2cb3011569 100644 --- a/java/ql/lib/ext/generated/org.apache.commons.lang.model.yml +++ b/java/ql/lib/ext/generated/org.apache.commons.lang.model.yml @@ -1604,8 +1604,6 @@ extensions: - ["org.apache.commons.lang", "SerializationUtils", "serialize", "(Serializable,OutputStream)", "summary", "df-generated"] - ["org.apache.commons.lang", "StringEscapeUtils", "escapeCsv", "(String)", "summary", "df-generated"] - ["org.apache.commons.lang", "StringEscapeUtils", "escapeCsv", "(Writer,String)", "summary", "df-generated"] - - ["org.apache.commons.lang", "StringEscapeUtils", "escapeHtml", "(String)", "summary", "df-generated"] - - ["org.apache.commons.lang", "StringEscapeUtils", "escapeHtml", "(Writer,String)", "summary", "df-generated"] - ["org.apache.commons.lang", "StringEscapeUtils", "escapeJava", "(String)", "summary", "df-generated"] - ["org.apache.commons.lang", "StringEscapeUtils", "escapeJava", "(Writer,String)", "summary", "df-generated"] - ["org.apache.commons.lang", "StringEscapeUtils", "escapeJavaScript", "(String)", "summary", "df-generated"] From 4eb1035dfef2773477178367be1955c9c970e0aa Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 1 Aug 2023 15:05:50 -0400 Subject: [PATCH 23/24] Documentation fixes --- .../customizing-library-models-for-java.rst | 2 +- .../change-notes/2023-07-25-trust-boundary-violation-query.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst b/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst index 05f7f9958d2..707ae531fc1 100644 --- a/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst +++ b/docs/codeql/codeql-language-guides/customizing-library-models-for-java.rst @@ -349,7 +349,7 @@ The following sink kinds are supported: - **response-splitting**: A sink that can be used for HTTP response splitting, such as in calls to **HttpServletResponse.setHeader**. - **sql-injection**: A sink that can be used for SQL injection, such as in a **Statement.executeQuery** call. - **template-injection**: A sink that can be used for server side template injection, such as in a **Velocity.evaluate** call. -- **trust-boundary-violation**: A sink that can be used to cross a trust boundary, such as a server's HTTP Session. +- **trust-boundary-violation**: A sink that can be used to cross a trust boundary, such as in a **HttpSession.setAttribute** call. - **url-redirection**: A sink that can be used to redirect the user to a malicious URL, such as in a **Response.temporaryRedirect** call. - **xpath-injection**: A sink that can be used for XPath injection, such as in a **XPath.evaluate** call. - **xslt-injection**: A sink that can be used for XSLT injection, such as in a **Transformer.transform** call. diff --git a/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md b/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md index df2e8aecf79..802e367bf10 100644 --- a/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md +++ b/java/ql/src/change-notes/2023-07-25-trust-boundary-violation-query.md @@ -1,5 +1,5 @@ --- category: newQuery --- -* Added the `java/trust-boundary-violation` query to detect trust boundary violations between HTTP requests and the HTTP session. +* Added the `java/trust-boundary-violation` query to detect trust boundary violations between HTTP requests and the HTTP session. Also added the `trust-boundary-violation` sink kind for sinks which may cross a trust boundary, such as calls to the `HttpSession#setAttribute` method. From 8d88af1af0666be96a950658301a366c834a9946 Mon Sep 17 00:00:00 2001 From: Edward Minnix III Date: Mon, 7 Aug 2023 00:37:13 -0400 Subject: [PATCH 24/24] Apply docs review suggestions Co-authored-by: Sam Browning <106113886+sabrowning1@users.noreply.github.com> --- .../src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp index e3491e9bcf8..f0eb5de2f7f 100644 --- a/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp +++ b/java/ql/src/Security/CWE/CWE-501/TrustBoundaryViolation.qhelp @@ -22,18 +22,18 @@

    - In order to maintain a trust boundary, data from less trusted sources should be validated before being used. + To maintain a trust boundary, validate data from less trusted sources before use.

    - In the first (bad) example, the server accepts a parameter from the user and uses it to set the username without validation. + In the first (bad) example, the server accepts a parameter from the user, then uses it to set the username without validation.

    - In the second (good) example, the server validates the parameter before using it to set the username. + In the second (good) example, the server validates the parameter from the user, then uses it to set the username.