Merge pull request #14472 from egregius313/egregius313/sync-local-and…

…-remote-queries Java: Synchronize `*Local` versions of queries with their remote counterpart
github · Oct 16, 2023 · 21bea38 · 21bea38
2 parents 822f371 + 31c04b5
commit 21bea38
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 4 deletions.
diff --git a/...ql/lib/change-notes/2023-10-12-sync-local-and-remote-dataflow-configurations.md b/...ql/lib/change-notes/2023-10-12-sync-local-and-remote-dataflow-configurations.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `isBarrier`, `isBarrierIn`, `isBarrierOut`, and `isAdditionalFlowStep` methods of the taint-tracking configurations for local queries in the `ArithmeticTaintedLocalQuery`, `ExternallyControlledFormatStringLocalQuery`, `ImproperValidationOfArrayIndexQuery`, `NumericCastTaintedQuery`, `ResponseSplittingLocalQuery`, `SqlTaintedLocalQuery`, and `XssLocalQuery` libraries have been changed to match their remote counterpart configurations.
diff --git a/java/ql/lib/semmle/code/java/security/ArithmeticTaintedLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ArithmeticTaintedLocalQuery.qll
@@ -13,6 +13,8 @@ module ArithmeticTaintedLocalOverflowConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) }
 
   predicate isBarrier(DataFlow::Node n) { overflowBarrier(n) }
+
+  predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
 }
 
 /**
@@ -30,6 +32,8 @@ module ArithmeticTaintedLocalUnderflowConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) }
 
   predicate isBarrier(DataFlow::Node n) { underflowBarrier(n) }
+
+  predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
 }
 
 /**

diff --git a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringLocalQuery.qll
@@ -11,6 +11,10 @@ module ExternallyControlledFormatStringLocalConfig implements DataFlow::ConfigSi
   predicate isSink(DataFlow::Node sink) {
     sink.asExpr() = any(StringFormat formatCall).getFormatArgument()
   }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.getType() instanceof NumericType or node.getType() instanceof BooleanType
+  }
 }
 
 /**

diff --git a/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayIndexLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayIndexLocalQuery.qll
@@ -13,6 +13,10 @@ module ImproperValidationOfArrayIndexLocalConfig implements DataFlow::ConfigSig
   predicate isSink(DataFlow::Node sink) {
     any(CheckableArrayAccess caa).canThrowOutOfBounds(sink.asExpr())
   }
+
+  predicate isBarrier(DataFlow::Node node) { node.getType() instanceof BooleanType }
+
+  predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
 }
 
 /**

diff --git a/java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll b/java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll
@@ -117,16 +117,20 @@ module NumericCastLocalFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
 
   predicate isSink(DataFlow::Node sink) {
-    sink.asExpr() = any(NumericNarrowingCastExpr cast).getExpr()
+    sink.asExpr() = any(NumericNarrowingCastExpr cast).getExpr() and
+    sink.asExpr() instanceof VarAccess
   }
 
   predicate isBarrier(DataFlow::Node node) {
     boundedRead(node.asExpr()) or
     castCheck(node.asExpr()) or
     node.getType() instanceof SmallType or
     smallExpr(node.asExpr()) or
-    node.getEnclosingCallable() instanceof HashCodeMethod
+    node.getEnclosingCallable() instanceof HashCodeMethod or
+    exists(RightShiftOp e | e.getShiftedVariable().getAnAccess() = node.asExpr())
   }
+
+  predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
 }
 
 /**

diff --git a/java/ql/lib/semmle/code/java/security/ResponseSplittingLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ResponseSplittingLocalQuery.qll
@@ -13,8 +13,21 @@ module ResponseSplittingLocalConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink }
 
   predicate isBarrier(DataFlow::Node node) {
-    node.getType() instanceof PrimitiveType or
+    node.getType() instanceof PrimitiveType
+    or
     node.getType() instanceof BoxedType
+    or
+    exists(MethodAccess ma, string methodName, CompileTimeConstantExpr target |
+      node.asExpr() = ma and
+      ma.getMethod().hasQualifiedName("java.lang", "String", methodName) and
+      target = ma.getArgument(0) and
+      (
+        methodName = "replace" and target.getIntValue() = [10, 13] // 10 == "\n", 13 == "\r"
+        or
+        methodName = "replaceAll" and
+        target.getStringValue().regexpMatch(".*([\n\r]|\\[\\^[^\\]\r\n]*\\]).*")
+      )
+    )
   }
 }
 

diff --git a/java/ql/lib/semmle/code/java/security/SqlTaintedLocalQuery.qll b/java/ql/lib/semmle/code/java/security/SqlTaintedLocalQuery.qll
@@ -17,7 +17,9 @@ module LocalUserInputToQueryInjectionFlowConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
 
   predicate isBarrier(DataFlow::Node node) {
-    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
+    node.getType() instanceof PrimitiveType or
+    node.getType() instanceof BoxedType or
+    node.getType() instanceof NumberType
   }
 
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {

diff --git a/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll b/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll
@@ -12,6 +12,14 @@ module XssLocalConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
 
   predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof XssSanitizer }
+
+  predicate isBarrierOut(DataFlow::Node node) { node instanceof XssSinkBarrier }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(XssAdditionalTaintStep s).step(node1, node2)
+  }
 }
 
 /**