Allow MaD sanitizers for java/sql-injection, java/concatenated-sql-query and java/ip-address-spoofing

Author: owen-mc

java/ql/lib/semmle/code/java/security/QueryInjection.qll

@@ -8,10 +8,14 @@ import semmle.code.java.frameworks.javaee.Persistence
 private import semmle.code.java.frameworks.MyBatis
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSinks
+private import semmle.code.java.security.Sanitizers
 
 /** A sink for database query language injection vulnerabilities. */
 abstract class QueryInjectionSink extends ApiSinkNode { }
 
+/** A sanitizer for query injection vulnerabilities. */
+abstract class QueryInjectionSanitizer extends DataFlow::Node { }
+
 /**
  * A unit class for adding additional taint steps.
  *
@@ -60,6 +64,13 @@ private class MongoDbInjectionSink extends QueryInjectionSink {
   }
 }
 
+private class SimpleTypeQueryInjectionSanitizer extends QueryInjectionSanitizer instanceof SimpleTypeSanitizer
+{ }
+
+private class ExternalQueryInjectionSanitizer extends QueryInjectionSanitizer {
+  ExternalQueryInjectionSanitizer() { barrierNode(this, "sql-injection") }
+}
+
 private class MongoJsonStep extends AdditionalQueryInjectionTaintStep {
   override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
     exists(MethodCall ma |

java/ql/lib/semmle/code/java/security/SqlConcatenatedQuery.qll

@@ -4,7 +4,6 @@ import java
 private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.security.SqlConcatenatedLib
 private import semmle.code.java.security.SqlInjectionQuery
-private import semmle.code.java.security.Sanitizers
 
 private class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
   UncontrolledStringBuilderSource() {
@@ -23,7 +22,7 @@ module UncontrolledStringBuilderSourceFlowConfig implements DataFlow::ConfigSig
 
   predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
 
-  predicate isBarrier(DataFlow::Node node) { node instanceof SimpleTypeSanitizer }
+  predicate isBarrier(DataFlow::Node node) { node instanceof QueryInjectionSanitizer }
 
   predicate observeDiffInformedIncrementalMode() { any() }
 

java/ql/lib/semmle/code/java/security/SqlInjectionQuery.qll

@@ -8,7 +8,6 @@
 
 import java
 import semmle.code.java.dataflow.FlowSources
-private import semmle.code.java.security.Sanitizers
 import semmle.code.java.security.QueryInjection
 
 /**
@@ -19,7 +18,7 @@ module QueryInjectionFlowConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
 
-  predicate isBarrier(DataFlow::Node node) { node instanceof SimpleTypeSanitizer }
+  predicate isBarrier(DataFlow::Node node) { node instanceof QueryInjectionSanitizer }
 
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     any(AdditionalQueryInjectionTaintStep s).step(node1, node2)

java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql

@@ -14,7 +14,6 @@
 import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.Sanitizers
 deprecated import ClientSuppliedIpUsedInSecurityCheckLib
 deprecated import ClientSuppliedIpUsedInSecurityCheckFlow::PathGraph
 
@@ -28,18 +27,8 @@ deprecated module ClientSuppliedIpUsedInSecurityCheckConfig implements DataFlow:
 
   predicate isSink(DataFlow::Node sink) { sink instanceof ClientSuppliedIpUsedInSecurityCheckSink }
 
-  /**
-   * Splitting a header value by `,` and taking an entry other than the first is sanitizing, because
-   * later entries may originate from more-trustworthy intermediate proxies, not the original client.
-   */
   predicate isBarrier(DataFlow::Node node) {
-    exists(ArrayAccess aa, MethodCall ma | aa.getArray() = ma |
-      ma.getQualifier() = node.asExpr() and
-      ma.getMethod() instanceof SplitMethod and
-      not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0
-    )
-    or
-    node instanceof SimpleTypeSanitizer
+    node instanceof ClientSuppliedIpUsedInSecurityCheckSanitizer
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll

@@ -4,6 +4,7 @@ import java
 import DataFlow
 import semmle.code.java.frameworks.Networking
 import semmle.code.java.security.QueryInjection
+import semmle.code.java.security.Sanitizers
 
 /**
  * A data flow source of the client ip obtained according to the remote endpoint identifier specified
@@ -28,6 +29,9 @@ class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::Node {
 /** A data flow sink for ip address forgery vulnerabilities. */
 abstract class ClientSuppliedIpUsedInSecurityCheckSink extends DataFlow::Node { }
 
+/** A data flow sanitizer for ip address forgery vulnerabilities. */
+abstract class ClientSuppliedIpUsedInSecurityCheckSanitizer extends DataFlow::Node { }
+
 /**
  * A data flow sink for remote client ip comparison.
  *
@@ -98,3 +102,23 @@ string getIpAddressRegex() {
   result =
     "^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$"
 }
+
+/**
+ * Splitting a header value by `,` and taking an entry other than the first is sanitizing, because
+ * later entries may originate from more-trustworthy intermediate proxies, not the original client.
+ */
+private class EntryAfterFirstSanitizer extends ClientSuppliedIpUsedInSecurityCheckSanitizer {
+  EntryAfterFirstSanitizer() {
+    exists(ArrayAccess aa, MethodCall ma | aa.getArray() = ma |
+      ma.getQualifier() = this.asExpr() and
+      ma.getMethod() instanceof SplitMethod and
+      not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0
+    )
+  }
+}
+
+private class SimpleTypeAddressSpoofingSanitizer extends ClientSuppliedIpUsedInSecurityCheckSanitizer instanceof SimpleTypeSanitizer
+{ }
+
+private class SqlOperationSanitizer extends ClientSuppliedIpUsedInSecurityCheckSanitizer instanceof QueryInjectionSanitizer
+{ }