Merge pull request #7002 from aschackmull/java/field-node

Java: Add FieldValueNode to break up cartesian step relation.

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll

@@ -8,9 +8,9 @@ private import semmle.code.java.dispatch.VirtualDispatch as VirtualDispatch
 private module DispatchImpl {
   /** Gets a viable implementation of the target of the given `Call`. */
   DataFlowCallable viableCallable(DataFlowCall c) {
-    result = VirtualDispatch::viableCallable(c.asCall())
+    result.asCallable() = VirtualDispatch::viableCallable(c.asCall())
     or
-    result.(SummarizedCallable) = c.asCall().getCallee().getSourceDeclaration()
+    result.(SummarizedCallable).asCallable() = c.asCall().getCallee().getSourceDeclaration()
   }
 
   /**
@@ -93,31 +93,32 @@ private module DispatchImpl {
    * qualifier is a parameter of the enclosing callable `c`.
    */
   predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
-    mayBenefitFromCallContext(call.asCall(), c, _)
+    mayBenefitFromCallContext(call.asCall(), c.asCallable(), _)
   }
 
   /**
    * Gets a viable dispatch target of `call` in the context `ctx`. This is
    * restricted to those `call`s for which a context might make a difference.
    */
-  Method viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
+  DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
     result = viableCallable(call) and
     exists(int i, Callable c, Method def, RefType t, boolean exact, MethodAccess ma |
       ma = call.asCall() and
       mayBenefitFromCallContext(ma, c, i) and
-      c = viableCallable(ctx) and
+      c = viableCallable(ctx).asCallable() and
       contextArgHasType(ctx.asCall(), i, t, exact) and
       ma.getMethod().getSourceDeclaration() = def
     |
-      exact = true and result = VirtualDispatch::exactMethodImpl(def, t.getSourceDeclaration())
+      exact = true and
+      result.asCallable() = VirtualDispatch::exactMethodImpl(def, t.getSourceDeclaration())
       or
       exact = false and
       exists(RefType t2 |
-        result = VirtualDispatch::viableMethodImpl(def, t.getSourceDeclaration(), t2) and
+        result.asCallable() = VirtualDispatch::viableMethodImpl(def, t.getSourceDeclaration(), t2) and
         not failsUnification(t, t2)
       )
       or
-      result = def and def instanceof SummarizedCallable
+      result.asCallable() = def and result instanceof SummarizedCallable
     )
   }
 

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll

@@ -14,14 +14,14 @@ newtype TNode =
     not e.getParent*() instanceof Annotation
   } or
   TExplicitParameterNode(Parameter p) {
-    exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable
+    exists(p.getCallable().getBody()) or p.getCallable() = any(SummarizedCallable sc).asCallable()
   } or
   TImplicitVarargsArray(Call c) {
     c.getCallee().isVarargs() and
     not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray())
   } or
   TInstanceParameterNode(Callable c) {
-    (exists(c.getBody()) or c instanceof SummarizedCallable) and
+    (exists(c.getBody()) or c = any(SummarizedCallable sc).asCallable()) and
     not c.isStatic()
   } or
   TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or
@@ -44,7 +44,8 @@ newtype TNode =
   } or
   TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
     FlowSummaryImpl::Private::summaryNodeRange(c, state)
-  }
+  } or
+  TFieldValueNode(Field f)
 
 private predicate explicitInstanceArgument(Call call, Expr instarg) {
   call instanceof MethodAccess and
@@ -94,19 +95,12 @@ module Public {
       result = this.(MallocNode).getClassInstanceExpr().getType()
       or
       result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getType()
+      or
+      result = this.(FieldValueNode).getField().getType()
     }
 
     /** Gets the callable in which this node occurs. */
-    Callable getEnclosingCallable() {
+    Callable getEnclosingCallable() { result = nodeGetEnclosingCallable(this).asCallable() }
-      result = this.asExpr().getEnclosingCallable() or
-      result = this.asParameter().getCallable() or
-      result = this.(ImplicitVarargsArray).getCall().getEnclosingCallable() or
-      result = this.(InstanceParameterNode).getCallable() or
-      result = this.(ImplicitInstanceAccess).getInstanceAccess().getEnclosingCallable() or
-      result = this.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
-      result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getEnclosingCallable() or
-      this = TSummaryInternalNode(result, _)
-    }
 
     private Type getImprovedTypeBound() {
       exprTypeFlow(this.asExpr(), result, _) or
@@ -257,6 +251,18 @@ module Public {
     abstract Node getPreUpdateNode();
   }
 
+  /**
+   * A node representing the value of a field.
+   */
+  class FieldValueNode extends Node, TFieldValueNode {
+    /** Gets the field corresponding to this node. */
+    Field getField() { this = TFieldValueNode(result) }
+
+    override string toString() { result = getField().toString() }
+
+    override Location getLocation() { result = getField().getLocation() }
+  }
+
   /**
    * Gets the node that occurs as the qualifier of `fa`.
    */
@@ -305,11 +311,21 @@ private class ImplicitExprPostUpdate extends ImplicitPostUpdateNode, TImplicitEx
 
 module Private {
   /** Gets the callable in which this node occurs. */
-  DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
+  DataFlowCallable nodeGetEnclosingCallable(Node n) {
+    result.asCallable() = n.asExpr().getEnclosingCallable() or
+    result.asCallable() = n.asParameter().getCallable() or
+    result.asCallable() = n.(ImplicitVarargsArray).getCall().getEnclosingCallable() or
+    result.asCallable() = n.(InstanceParameterNode).getCallable() or
+    result.asCallable() = n.(ImplicitInstanceAccess).getInstanceAccess().getEnclosingCallable() or
+    result.asCallable() = n.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
+    result = nodeGetEnclosingCallable(n.(ImplicitPostUpdateNode).getPreUpdateNode()) or
+    n = TSummaryInternalNode(result, _) or
+    result.asFieldScope() = n.(FieldValueNode).getField()
+  }
 
   /** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
   predicate isParameterNode(ParameterNode p, DataFlowCallable c, int pos) {
-    p.isParameterOf(c, pos)
+    p.isParameterOf(c.asCallable(), pos)
   }
 
   /**

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

@@ -32,12 +32,18 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
 /**
  * Holds if data can flow from `node1` to `node2` through a static field.
  */
-private predicate staticFieldStep(ExprNode node1, ExprNode node2) {
+private predicate staticFieldStep(Node node1, Node node2) {
+  exists(Field f |
+    f.isStatic() and
+    f.getAnAssignedValue() = node1.asExpr() and
+    node2.(FieldValueNode).getField() = f
+  )
+  or
   exists(Field f, FieldRead fr |
     f.isStatic() and
-    f.getAnAssignedValue() = node1.getExpr() and
+    node1.(FieldValueNode).getField() = f and
     fr.getField() = f and
-    fr = node2.getExpr() and
+    fr = node2.asExpr() and
     hasNonlocalValue(fr)
   )
 }
@@ -205,7 +211,30 @@ class CastNode extends ExprNode {
   CastNode() { this.getExpr() instanceof CastExpr }
 }
 
-class DataFlowCallable = Callable;
+private newtype TDataFlowCallable =
+  TCallable(Callable c) or
+  TFieldScope(Field f)
+
+class DataFlowCallable extends TDataFlowCallable {
+  Callable asCallable() { this = TCallable(result) }
+
+  Field asFieldScope() { this = TFieldScope(result) }
+
+  RefType getDeclaringType() {
+    result = asCallable().getDeclaringType() or
+    result = asFieldScope().getDeclaringType()
+  }
+
+  string toString() {
+    result = asCallable().toString() or
+    result = "Field scope: " + asFieldScope().toString()
+  }
+
+  Location getLocation() {
+    result = asCallable().getLocation() or
+    result = asFieldScope().getLocation()
+  }
+}
 
 class DataFlowExpr = Expr;
 
@@ -251,7 +280,9 @@ class SrcCall extends DataFlowCall, TCall {
 
   SrcCall() { this = TCall(call) }
 
-  override DataFlowCallable getEnclosingCallable() { result = call.getEnclosingCallable() }
+  override DataFlowCallable getEnclosingCallable() {
+    result.asCallable() = call.getEnclosingCallable()
+  }
 
   override string toString() { result = call.toString() }
 
@@ -345,10 +376,10 @@ class LambdaCallKind = Method; // the "apply" method in the functional interface
 predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) {
   exists(ClassInstanceExpr func, Interface t, FunctionalInterface interface |
     creation.asExpr() = func and
-    func.getAnonymousClass().getAMethod() = c and
+    func.getAnonymousClass().getAMethod() = c.asCallable() and
     func.getConstructedType().extendsOrImplements+(t) and
     t.getSourceDeclaration() = interface and
-    c.(Method).overridesOrInstantiates+(pragma[only_bind_into](kind)) and
+    c.asCallable().(Method).overridesOrInstantiates+(pragma[only_bind_into](kind)) and
     pragma[only_bind_into](kind) = interface.getRunMethod().getSourceDeclaration()
   )
 }

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -30,7 +30,7 @@ DataFlowType getContentType(Content c) { result = c.getType() }
 
 /** Gets the return type of kind `rk` for callable `c`. */
 DataFlowType getReturnType(SummarizedCallable c, ReturnKind rk) {
-  result = getErasedRepr(c.getReturnType()) and
+  result = getErasedRepr(c.asCallable().getReturnType()) and
   exists(rk)
 }
 
@@ -62,7 +62,7 @@ predicate summaryElement(DataFlowCallable c, string input, string output, string
     string namespace, string type, boolean subtypes, string name, string signature, string ext
   |
     summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind) and
-    c = interpretElement(namespace, type, subtypes, name, signature, ext)
+    c.asCallable() = interpretElement(namespace, type, subtypes, name, signature, ext)
   )
 }
 
@@ -119,7 +119,7 @@ class InterpretNode extends TInterpretNode {
   DataFlowCall asCall() { result.asCall() = this.asElement() }
 
   /** Gets the callable that this node corresponds to, if any. */
-  DataFlowCallable asCallable() { result = this.asElement() }
+  DataFlowCallable asCallable() { result.asCallable() = this.asElement() }
 
   /** Gets the target of this call, if any. */
   Callable getCallTarget() { result = this.asCall().asCall().getCallee().getSourceDeclaration() }

java/ql/lib/semmle/code/java/dispatch/DispatchFlow.qll

@@ -189,10 +189,9 @@ private predicate flowStep(RelevantNode n1, RelevantNode n2) {
     n2.(ImplicitInstanceAccess).getInstanceAccess().(OwnInstanceAccess).getEnclosingCallable() = c
   )
   or
-  exists(Field f |
+  n2.(FieldValueNode).getField().getAnAssignedValue() = n1.asExpr()
-    f.getAnAssignedValue() = n1.asExpr() and
+  or
-    n2.asExpr().(FieldRead).getField() = f
+  n2.asExpr().(FieldRead).getField() = n1.(FieldValueNode).getField()
-  )
   or
   exists(EnumType enum, Method getValue |
     enum.getAnEnumConstant().getAnAssignedValue() = n1.asExpr() and

java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll

@@ -94,10 +94,9 @@ private predicate step(Node n1, Node n2) {
     n2.(ImplicitInstanceAccess).getInstanceAccess().(OwnInstanceAccess).getEnclosingCallable() = c
   )
   or
-  exists(Field f |
+  n2.(FieldValueNode).getField().getAnAssignedValue() = n1.asExpr()
-    f.getAnAssignedValue() = n1.asExpr() and
+  or
-    n2.asExpr().(FieldRead).getField() = f
+  n2.asExpr().(FieldRead).getField() = n1.(FieldValueNode).getField()
-  )
   or
   n2.asExpr().(CastExpr).getExpr() = n1.asExpr()
   or
@@ -132,7 +131,7 @@ private predicate step(Node n1, Node n2) {
   or
   exists(Field v |
     containerStep(n1.asExpr(), v.getAnAccess()) and
-    n2.asExpr() = v.getAnAccess()
+    n2.(FieldValueNode).getField() = v
   )
 }
 

java/ql/src/Telemetry/ExternalAPI.qll

@@ -62,7 +62,7 @@ class ExternalAPI extends Callable {
 
   /** Holds if this API has a supported summary. */
   predicate hasSummary() {
-    this instanceof SummarizedCallable or
+    this = any(SummarizedCallable sc).asCallable() or
     TaintTracking::localAdditionalTaintStep(this.getAnInput(), _)
   }
 

java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected

@@ -1,7 +1,8 @@
 edges
 | UnsafeHostnameVerification.java:66:37:80:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:81:55:81:62 | verifier |
 | UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:94:55:94:62 | verifier |
-| UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:34:59:34:85 | ALLOW_ALL_HOSTNAME_VERIFIER |
+| UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:34:59:34:85 | ALLOW_ALL_HOSTNAME_VERIFIER |
+| UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } |
 nodes
 | UnsafeHostnameVerification.java:14:55:19:9 | new (...) | semmle.label | new (...) |
 | UnsafeHostnameVerification.java:26:55:26:71 | ...->... | semmle.label | ...->... |
@@ -12,6 +13,7 @@ nodes
 | UnsafeHostnameVerification.java:81:55:81:62 | verifier | semmle.label | verifier |
 | UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | semmle.label | new (...) : new HostnameVerifier(...) { ... } |
 | UnsafeHostnameVerification.java:94:55:94:62 | verifier | semmle.label | verifier |
+| UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } | semmle.label | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } |
 | UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | semmle.label | new (...) : new HostnameVerifier(...) { ... } |
 subpaths
 #select

java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected

@@ -1,6 +1,7 @@
 edges
-| CredentialsTest.java:7:34:7:41 | "123456" : String | CredentialsTest.java:13:39:13:39 | p |
+| CredentialsTest.java:7:30:7:30 | p : String | CredentialsTest.java:13:39:13:39 | p |
-| CredentialsTest.java:7:34:7:41 | "123456" : String | CredentialsTest.java:14:16:14:16 | p : String |
+| CredentialsTest.java:7:30:7:30 | p : String | CredentialsTest.java:14:16:14:16 | p : String |
+| CredentialsTest.java:7:34:7:41 | "123456" : String | CredentialsTest.java:7:30:7:30 | p : String |
 | CredentialsTest.java:11:14:11:20 | "admin" : String | CredentialsTest.java:13:36:13:36 | u |
 | CredentialsTest.java:11:14:11:20 | "admin" : String | CredentialsTest.java:14:13:14:13 | u : String |
 | CredentialsTest.java:14:13:14:13 | u : String | CredentialsTest.java:17:38:17:45 | v : String |
@@ -44,6 +45,7 @@ edges
 | Test.java:29:38:29:48 | user : String | Test.java:30:36:30:39 | user |
 | Test.java:29:51:29:65 | password : String | Test.java:30:42:30:49 | password |
 nodes
+| CredentialsTest.java:7:30:7:30 | p : String | semmle.label | p : String |
 | CredentialsTest.java:7:34:7:41 | "123456" : String | semmle.label | "123456" : String |
 | CredentialsTest.java:11:14:11:20 | "admin" : String | semmle.label | "admin" : String |
 | CredentialsTest.java:13:36:13:36 | u | semmle.label | u |

java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsSourceCall.expected

@@ -12,7 +12,8 @@ edges
 | HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [clientSecret] : String |
 | HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [username] : String |
 | Test.java:10:17:10:24 | "123456" : String | Test.java:26:17:26:20 | pass |
-| User.java:2:43:2:50 | "123456" : String | User.java:5:15:5:24 | DEFAULT_PW |
+| User.java:2:30:2:39 | DEFAULT_PW : String | User.java:5:15:5:24 | DEFAULT_PW |
+| User.java:2:43:2:50 | "123456" : String | User.java:2:30:2:39 | DEFAULT_PW : String |
 nodes
 | HardcodedAzureCredentials.java:8:14:8:38 | this <.method> [post update] [clientSecret] : String | semmle.label | this <.method> [post update] [clientSecret] : String |
 | HardcodedAzureCredentials.java:8:14:8:38 | this <.method> [post update] [username] : String | semmle.label | this <.method> [post update] [username] : String |
@@ -30,6 +31,7 @@ nodes
 | HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | semmle.label | new HardcodedAzureCredentials(...) [username] : String |
 | Test.java:10:17:10:24 | "123456" : String | semmle.label | "123456" : String |
 | Test.java:26:17:26:20 | pass | semmle.label | pass |
+| User.java:2:30:2:39 | DEFAULT_PW : String | semmle.label | DEFAULT_PW : String |
 | User.java:2:43:2:50 | "123456" : String | semmle.label | "123456" : String |
 | User.java:5:15:5:24 | DEFAULT_PW | semmle.label | DEFAULT_PW |
 subpaths