Model value passing between a setter and a getter call as a value step

Author: luchua-bc

java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql

@@ -28,6 +28,10 @@ class InjectFilePathConfig extends TaintTracking::Configuration {
     not sink instanceof NormalizedPathNode
   }
 
+  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    any(AdditionalValueStep r).step(pred, succ)
+  }
+
   override predicate isSanitizer(DataFlow::Node node) {
     exists(Type t | t = node.getType() | t instanceof BoxedType or t instanceof PrimitiveType)
   }

java/ql/src/experimental/Security/CWE/CWE-073/JFinalController.qll

@@ -38,32 +38,25 @@ class SetRequestAttributeMethod extends Method {
   }
 }
 
-/**
- * Holds if the result of an attribute getter call is from a method invocation of remote attribute setter.
- * Only values received from remote flow source is to be checked by the query.
- */
-predicate isGetAttributeFromRemoteSource(Expr expr) {
-  exists(MethodAccess gma, MethodAccess sma |
-    (
-      gma.getMethod() instanceof GetSessionAttributeMethod and
-      sma.getMethod() instanceof SetSessionAttributeMethod
-      or
-      gma.getMethod() instanceof GetRequestAttributeMethod and
-      sma.getMethod() instanceof SetRequestAttributeMethod
-    ) and
-    expr = gma and
-    gma.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
-      sma.getArgument(0).(CompileTimeConstantExpr).getStringValue() and
-    gma.getEnclosingCallable() = sma.getEnclosingCallable() and
-    TaintTracking::localExprTaint(any(RemoteFlowSource rs).asExpr(), sma.getArgument(1))
-  )
-}
-
-/** Remote flow source of JFinal request or session attribute getters. */
-private class JFinalRequestSource extends RemoteFlowSource {
-  JFinalRequestSource() { isGetAttributeFromRemoteSource(this.asExpr()) }
-
-  override string getSourceType() { result = "JFinal session or request attribute source" }
+/** Value step from the setter call to the getter call of a session or request attribute. */
+private class SetToGetAttributeStep extends AdditionalValueStep {
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(MethodAccess gma, MethodAccess sma |
+      (
+        gma.getMethod() instanceof GetSessionAttributeMethod and
+        sma.getMethod() instanceof SetSessionAttributeMethod
+        or
+        gma.getMethod() instanceof GetRequestAttributeMethod and
+        sma.getMethod() instanceof SetRequestAttributeMethod
+      ) and
+      gma.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
+        sma.getArgument(0).(CompileTimeConstantExpr).getStringValue() and
+      gma.getEnclosingCallable() = sma.getEnclosingCallable()
+    |
+      pred.asExpr() = sma.getArgument(1) and
+      succ.asExpr() = gma
+    )
+  }
 }
 
 /** Source model of remote flow source with `JFinal`. */

java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected

@@ -1,20 +1,20 @@
 edges
 | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath |
-| FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath |
-| FilePathInjection.java:89:29:89:48 | getAttr(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath |
+| FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath |
+| FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath |
 | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath |
 nodes
 | FilePathInjection.java:21:21:21:34 | getPara(...) : String | semmle.label | getPara(...) : String |
 | FilePathInjection.java:26:47:26:59 | finalFilePath | semmle.label | finalFilePath |
-| FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | semmle.label | getSessionAttr(...) : String |
+| FilePathInjection.java:64:21:64:34 | getPara(...) : String | semmle.label | getPara(...) : String |
 | FilePathInjection.java:72:47:72:59 | finalFilePath | semmle.label | finalFilePath |
-| FilePathInjection.java:89:29:89:48 | getAttr(...) : String | semmle.label | getAttr(...) : String |
+| FilePathInjection.java:87:21:87:34 | getPara(...) : String | semmle.label | getPara(...) : String |
 | FilePathInjection.java:95:47:95:59 | finalFilePath | semmle.label | finalFilePath |
 | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
 | FilePathInjection.java:209:24:209:31 | filePath | semmle.label | filePath |
 subpaths
 #select
 | FilePathInjection.java:26:47:26:59 | finalFilePath | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:21:21:21:34 | getPara(...) | user-provided value |
-| FilePathInjection.java:72:47:72:59 | finalFilePath | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) | user-provided value |
-| FilePathInjection.java:95:47:95:59 | finalFilePath | FilePathInjection.java:89:29:89:48 | getAttr(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:89:29:89:48 | getAttr(...) | user-provided value |
+| FilePathInjection.java:72:47:72:59 | finalFilePath | FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:64:21:64:34 | getPara(...) | user-provided value |
+| FilePathInjection.java:95:47:95:59 | finalFilePath | FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:87:21:87:34 | getPara(...) | user-provided value |
 | FilePathInjection.java:209:24:209:31 | filePath | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:205:17:205:44 | getParameter(...) | user-provided value |