diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll index 0e61dc4582f..91b8b391df5 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll +++ b/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 ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll index 70f4f38f134..3509b38108e 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll +++ b/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() { - 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, _) - } + Callable getEnclosingCallable() { result = nodeGetEnclosingCallable(this).asCallable() } 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) } /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index d0df4c33cf5..89c53c8854e 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/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() ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll index e4c3091f05e..a3d5d64a766 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll +++ b/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() } diff --git a/java/ql/lib/semmle/code/java/dispatch/DispatchFlow.qll b/java/ql/lib/semmle/code/java/dispatch/DispatchFlow.qll index bbb516d3bf0..1b13e6ef4c0 100644 --- a/java/ql/lib/semmle/code/java/dispatch/DispatchFlow.qll +++ b/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 | - f.getAnAssignedValue() = n1.asExpr() and - n2.asExpr().(FieldRead).getField() = f - ) + n2.(FieldValueNode).getField().getAnAssignedValue() = n1.asExpr() + or + n2.asExpr().(FieldRead).getField() = n1.(FieldValueNode).getField() or exists(EnumType enum, Method getValue | enum.getAnEnumConstant().getAnAssignedValue() = n1.asExpr() and diff --git a/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll b/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll index 045ce76e9af..005b54a9259 100644 --- a/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll +++ b/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 | - f.getAnAssignedValue() = n1.asExpr() and - n2.asExpr().(FieldRead).getField() = f - ) + n2.(FieldValueNode).getField().getAnAssignedValue() = n1.asExpr() + or + 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 ) } diff --git a/java/ql/src/Telemetry/ExternalAPI.qll b/java/ql/src/Telemetry/ExternalAPI.qll index 76ed5734f3f..5f41ff14d3a 100644 --- a/java/ql/src/Telemetry/ExternalAPI.qll +++ b/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(), _) } diff --git a/java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected b/java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected index 405b02b4877..13a14d69756 100644 --- a/java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected +++ b/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 diff --git a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected index 33b8879d9e5..96cdab78eb8 100644 --- a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected +++ b/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:34:7:41 | "123456" : String | CredentialsTest.java:14:16:14:16 | p : String | +| CredentialsTest.java:7:30:7:30 | p : String | CredentialsTest.java:13:39:13:39 | p | +| 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 | diff --git a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsSourceCall.expected b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsSourceCall.expected index a960464f0d9..bf73fdaa93d 100644 --- a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsSourceCall.expected +++ b/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