From 5abf7769a77462b916418ab04d514e334b07632e Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Fri, 4 Aug 2023 15:15:42 +0200 Subject: [PATCH 1/8] Java: automodel application mode: use endpoint class like in framework mode --- ...utomodelApplicationModeCharacteristics.qll | 163 ++++++++++-------- ...tomodelApplicationModeExtractCandidates.ql | 5 +- ...lApplicationModeExtractNegativeExamples.ql | 5 +- ...lApplicationModeExtractPositiveExamples.ql | 6 +- ...lApplicationModeExtractCandidates.expected | 5 +- ...cationModeExtractNegativeExamples.expected | 4 +- ...cationModeExtractPositiveExamples.expected | 6 +- .../Test.java | 9 +- 8 files changed, 119 insertions(+), 84 deletions(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll index e14c18ace326..2cd267801654 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll +++ b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll @@ -21,21 +21,81 @@ import AutomodelEndpointTypes as AutomodelEndpointTypes newtype JavaRelatedLocationType = CallContext() +newtype TApplicationModeEndpoint = + TExplicitArgument(Call call, DataFlow::Node arg) { + exists(Argument argExpr | + arg.asExpr() = argExpr and not argExpr.isVararg() and call = argExpr.getCall() + ) + } or + TInstanceArgument(Call call, DataFlow::Node arg) { arg = DataFlow::getInstanceArgument(call) } or + TImplicitVarargsArray(Call call, DataFlow::ImplicitVarargsArray varargs, int idx) { + varargs.getCall() = call and + idx = min(Argument arg, int n | arg = call.getArgument(n) and arg.isVararg() | n) + } + +abstract private class ApplicationModeEndpoint extends TApplicationModeEndpoint { + abstract predicate isArgOf(Call c, int idx); + + Call getCall() { this.isArgOf(result, _) } + + int getArgIndex() { this.isArgOf(_, result) } + + abstract Top asTop(); + + abstract DataFlow::Node asNode(); + + abstract string toString(); +} + /** * A class representing nodes that are arguments to calls. */ -private class ArgumentNode extends DataFlow::Node { - Call c; +class ExplicitArgument extends ApplicationModeEndpoint, TExplicitArgument { + Call call; + DataFlow::Node arg; - ArgumentNode() { - exists(Argument arg | this.asExpr() = arg and not arg.isVararg() and c = arg.getCall()) - or - this.(DataFlow::ImplicitVarargsArray).getCall() = c - or - this = DataFlow::getInstanceArgument(c) + ExplicitArgument() { this = TExplicitArgument(call, arg) } + + override predicate isArgOf(Call c, int idx) { c = call and this.asTop() = c.getArgument(idx) } + + override Top asTop() { result = arg.asExpr() } + + override DataFlow::Node asNode() { result = arg } + + override string toString() { result = arg.toString() } +} + +class InstanceArgument extends ApplicationModeEndpoint, TInstanceArgument { + Call call; + DataFlow::Node arg; + + InstanceArgument() { this = TInstanceArgument(call, arg) } + + override predicate isArgOf(Call c, int idx) { + c = call and this.asTop() = c.getQualifier() and idx = -1 } - Call getCall() { result = c } + override Top asTop() { if exists(arg.asExpr()) then result = arg.asExpr() else result = call } + + override DataFlow::Node asNode() { result = arg } + + override string toString() { result = arg.toString() } +} + +class ImplicitVarargsArray extends ApplicationModeEndpoint, TImplicitVarargsArray { + Call call; + DataFlow::ImplicitVarargsArray varargs; + int idx; + + ImplicitVarargsArray() { this = TImplicitVarargsArray(call, varargs, idx) } + + override predicate isArgOf(Call c, int i) { c = call and i = idx } + + override Top asTop() { result = this.getCall() } + + override DataFlow::Node asNode() { result = varargs } + + override string toString() { result = varargs.toString() } } /** @@ -47,7 +107,7 @@ private class ArgumentNode extends DataFlow::Node { */ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig { // for documentation of the implementations here, see the QLDoc in the CandidateSig signature module. - class Endpoint = ArgumentNode; + class Endpoint = ApplicationModeEndpoint; class EndpointType = AutomodelEndpointTypes::EndpointType; @@ -61,18 +121,18 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig predicate isSanitizer(Endpoint e, EndpointType t) { exists(t) and ( - e.getType() instanceof BoxedType + e.asNode().getType() instanceof BoxedType or - e.getType() instanceof PrimitiveType + e.asNode().getType() instanceof PrimitiveType or - e.getType() instanceof NumberType + e.asNode().getType() instanceof NumberType ) or t instanceof AutomodelEndpointTypes::PathInjectionSinkType and - e instanceof PathSanitizer::PathInjectionSanitizer + e.asNode() instanceof PathSanitizer::PathInjectionSanitizer } - RelatedLocation asLocation(Endpoint e) { result = e.asExpr() } + RelatedLocation asLocation(Endpoint e) { result = e.asTop() } predicate isKnownKind = AutomodelJavaUtil::isKnownKind/2; @@ -98,16 +158,7 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig ApplicationModeGetCallable::getCallable(e).hasQualifiedName(package, type, name) and signature = ExternalFlow::paramsString(ApplicationModeGetCallable::getCallable(e)) and ext = "" and - ( - exists(Call c, int argIdx | - e.asExpr() = c.getArgument(argIdx) and - input = AutomodelJavaUtil::getArgumentForIndex(argIdx) - ) - or - exists(Call c | - e.asExpr() = c.getQualifier() and input = AutomodelJavaUtil::getArgumentForIndex(-1) - ) - ) + input = AutomodelJavaUtil::getArgumentForIndex(e.getArgIndex()) } /** @@ -118,7 +169,7 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig */ RelatedLocation getRelatedLocation(Endpoint e, RelatedLocationType type) { type = CallContext() and - result = any(Call c | e.asExpr() = [c.getAnArgument(), c.getQualifier()]) + result = e.asTop() } } @@ -132,12 +183,7 @@ private module ApplicationModeGetCallable implements AutomodelSharedGetCallable: /** * Returns the API callable being modeled. */ - Callable getCallable(Endpoint e) { - exists(Call c | - e.asExpr() = [c.getAnArgument(), c.getQualifier()] and - result = c.getCallee() - ) - } + Callable getCallable(Endpoint e) { result = e.getCall().getCallee() } } /** @@ -145,7 +191,7 @@ private module ApplicationModeGetCallable implements AutomodelSharedGetCallable: * should be empty. */ private predicate isCustomSink(Endpoint e, string kind) { - e instanceof QueryInjectionSink and kind = "sql" + e.asNode() instanceof QueryInjectionSink and kind = "sql" } module CharacteristicsImpl = @@ -169,14 +215,9 @@ class ApplicationModeMetadataExtractor extends string { Endpoint e, string package, string type, string subtypes, string name, string signature, string input ) { - exists(Call call, Callable callable, int argIdx | - call.getCallee() = callable and - ( - e.asExpr() = call.getArgument(argIdx) - or - e.asExpr() = call.getQualifier() and argIdx = -1 - ) and - input = AutomodelJavaUtil::getArgumentForIndex(argIdx) and + exists(Callable callable | + e.getCall().getCallee() = callable and + input = AutomodelJavaUtil::getArgumentForIndex(e.getArgIndex()) and package = callable.getDeclaringType().getPackage().getName() and // we're using the erased types because the MaD convention is to not specify type parameters. // Whether something is or isn't a sink doesn't usually depend on the type parameters. @@ -253,28 +294,10 @@ private class IsMaDTaintStepCharacteristic extends CharacteristicsImpl::NotASink IsMaDTaintStepCharacteristic() { this = "taint step" } override predicate appliesToEndpoint(Endpoint e) { - FlowSummaryImpl::Private::Steps::summaryThroughStepValue(e, _, _) or - FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(e, _, _) or - FlowSummaryImpl::Private::Steps::summaryGetterStep(e, _, _, _) or - FlowSummaryImpl::Private::Steps::summarySetterStep(e, _, _, _) - } -} - -/** - * A negative characteristic that filters out qualifiers that are classes (i.e. static calls). These - * are unlikely to have any non-trivial flow going into them. - * - * Technically, an accessed type _could_ come from outside of the source code, but there's not - * much likelihood of that being user-controlled. - */ -private class ClassQualifierCharacteristic extends CharacteristicsImpl::NotASinkCharacteristic { - ClassQualifierCharacteristic() { this = "class qualifier" } - - override predicate appliesToEndpoint(Endpoint e) { - exists(Call c | - e.asExpr() = c.getQualifier() and - c.getQualifier() instanceof TypeAccess - ) + FlowSummaryImpl::Private::Steps::summaryThroughStepValue(e.asNode(), _, _) or + FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(e.asNode(), _, _) or + FlowSummaryImpl::Private::Steps::summaryGetterStep(e.asNode(), _, _, _) or + FlowSummaryImpl::Private::Steps::summarySetterStep(e.asNode(), _, _, _) } } @@ -351,7 +374,7 @@ private class OtherArgumentToModeledMethodCharacteristic extends Characteristics private class FunctionValueCharacteristic extends CharacteristicsImpl::LikelyNotASinkCharacteristic { FunctionValueCharacteristic() { this = "function value" } - override predicate appliesToEndpoint(Endpoint e) { e.asExpr() instanceof FunctionalExpr } + override predicate appliesToEndpoint(Endpoint e) { e.asNode().asExpr() instanceof FunctionalExpr } } /** @@ -371,12 +394,12 @@ private class CannotBeTaintedCharacteristic extends CharacteristicsImpl::LikelyN * Holds if the node `n` is known as the predecessor in a modeled flow step. */ private predicate isKnownOutNodeForStep(Endpoint e) { - e.asExpr() instanceof Call or // we just assume flow in that case - TaintTracking::localTaintStep(_, e) or - FlowSummaryImpl::Private::Steps::summaryThroughStepValue(_, e, _) or - FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(_, e, _) or - FlowSummaryImpl::Private::Steps::summaryGetterStep(_, _, e, _) or - FlowSummaryImpl::Private::Steps::summarySetterStep(_, _, e, _) + e.asNode().asExpr() instanceof Call or // we just assume flow in that case + TaintTracking::localTaintStep(_, e.asNode()) or + FlowSummaryImpl::Private::Steps::summaryThroughStepValue(_, e.asNode(), _) or + FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(_, e.asNode(), _) or + FlowSummaryImpl::Private::Steps::summaryGetterStep(_, _, e.asNode(), _) or + FlowSummaryImpl::Private::Steps::summarySetterStep(_, _, e.asNode(), _) } } diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql index 4940b4a741ff..ec33b43b3d50 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql @@ -33,7 +33,7 @@ private Endpoint getSampleForSignature( | result = rank[n](Endpoint e, Location loc | - loc = e.getLocation() and + loc = e.asTop().getLocation() and meta.hasMetadata(e, package, type, subtypes, name, signature, input) | e @@ -75,7 +75,8 @@ where | sinkType, ", " ) -select endpoint, message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // +select endpoint.asNode(), + message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext()), "CallContext", // package, "package", // type, "type", // diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql index e8a284dd6c05..4022dbfdeda6 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql @@ -24,7 +24,7 @@ Endpoint getSampleForCharacteristic(EndpointCharacteristic c, int limit) { exists(int n, int num_endpoints | num_endpoints = count(Endpoint e | c.appliesToEndpoint(e)) | result = rank[n](Endpoint e, Location loc | - loc = e.getLocation() and c.appliesToEndpoint(e) + loc = e.asTop().getLocation() and c.appliesToEndpoint(e) | e order by @@ -63,7 +63,8 @@ where characteristic2.hasImplications(positiveType, true, confidence2) ) and message = characteristic -select endpoint, message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // +select endpoint.asNode(), + message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext()), "CallContext", // package, "package", // type, "type", // diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql index c62476377db4..99396ce40e26 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql @@ -22,8 +22,10 @@ where not erroneousEndpoints(endpoint, _, _, _, _, false) and meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input) and // Extract positive examples of sinks belonging to the existing ATM query configurations. - CharacteristicsImpl::isKnownSink(endpoint, sinkType) -select endpoint, sinkType + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // + CharacteristicsImpl::isKnownSink(endpoint, sinkType) and + exists(CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext())) +select endpoint.asNode(), + sinkType + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext()), "CallContext", // package, "package", // type, "type", // diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected index 8b6f95795b0e..71b20780172a 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected @@ -1,2 +1,3 @@ -| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:24 | set(...) | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | -| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:16 | get(...) | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | +| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | +| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | +| Test.java:51:3:54:3 | new ..[] { .. } | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:54:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected index 0c626ca2d0b2..9db66bdeed35 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected @@ -1,2 +1,2 @@ -| Test.java:40:14:40:21 | openPath | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:40:4:40:22 | get(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Paths:1:1:1:1 | Paths | type | file://false:1:1:1:1 | false | subtypes | file://get:1:1:1:1 | get | name | file://(String,String[]):1:1:1:1 | (String,String[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | -| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:45:10:47:3 | compareTo(...) | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | +| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:46:4:46:5 | f2 | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | +| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:52:4:52:4 | p | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected index 841599ce0217..ded3d5a9f97b 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected @@ -1,3 +1,3 @@ -| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:25:3:29:3 | copy(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | -| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:25:3:29:3 | copy(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | -| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:33:10:35:3 | newInputStream(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | +| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:26:4:26:9 | source | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | +| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:27:4:27:9 | target | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | +| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:34:4:34:11 | openPath | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java index 67423855a8bb..8991475134c7 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java @@ -8,7 +8,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.io.File; - +import java.nio.file.FileVisitOption; class Test { public static void main(String[] args) throws Exception { @@ -46,5 +46,12 @@ public static int compareFiles(File f1, File f2) { f2 // negative example (modeled as not a sink) ); } + + public static void FilesWalkExample(Path p) throws Exception { + Files.walk( + p, // negative example (modeled as a taint step) + FileVisitOption.FOLLOW_LINKS // the implicit varargs array is a candidate + ); + } } From 0781cb78e84d664dd91806d41bc12771bf93b3e3 Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Mon, 7 Aug 2023 11:04:44 +0200 Subject: [PATCH 2/8] Java: automodel application mode: add isVarargsArray metadata value --- ...AutomodelApplicationModeCharacteristics.qll | 7 +++++-- ...utomodelApplicationModeExtractCandidates.ql | 18 +++++++++++------- ...elApplicationModeExtractNegativeExamples.ql | 8 +++++--- ...elApplicationModeExtractPositiveExamples.ql | 7 ++++--- ...elApplicationModeExtractCandidates.expected | 6 +++--- ...icationModeExtractNegativeExamples.expected | 4 ++-- ...icationModeExtractPositiveExamples.expected | 6 +++--- 7 files changed, 33 insertions(+), 23 deletions(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll index 2cd267801654..4133bd5c8f12 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll +++ b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll @@ -213,7 +213,7 @@ class ApplicationModeMetadataExtractor extends string { predicate hasMetadata( Endpoint e, string package, string type, string subtypes, string name, string signature, - string input + string input, string isVarargsArray ) { exists(Callable callable | e.getCall().getCallee() = callable and @@ -224,7 +224,10 @@ class ApplicationModeMetadataExtractor extends string { type = callable.getDeclaringType().getErasure().(RefType).nestedName() and subtypes = AutomodelJavaUtil::considerSubtypes(callable).toString() and name = callable.getName() and - signature = ExternalFlow::paramsString(callable) + signature = ExternalFlow::paramsString(callable) and + if e instanceof ImplicitVarargsArray + then isVarargsArray = "true" + else isVarargsArray = "false" ) } } diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql index ec33b43b3d50..b0c3fe2a3f36 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql @@ -25,16 +25,18 @@ private import AutomodelJavaUtil bindingset[limit] private Endpoint getSampleForSignature( int limit, string package, string type, string subtypes, string name, string signature, - string input + string input, string isVarargs ) { exists(int n, int num_endpoints, ApplicationModeMetadataExtractor meta | num_endpoints = - count(Endpoint e | meta.hasMetadata(e, package, type, subtypes, name, signature, input)) + count(Endpoint e | + meta.hasMetadata(e, package, type, subtypes, name, signature, input, isVarargs) + ) | result = rank[n](Endpoint e, Location loc | loc = e.asTop().getLocation() and - meta.hasMetadata(e, package, type, subtypes, name, signature, input) + meta.hasMetadata(e, package, type, subtypes, name, signature, input, isVarargs) | e order by @@ -53,19 +55,20 @@ private Endpoint getSampleForSignature( from Endpoint endpoint, string message, ApplicationModeMetadataExtractor meta, DollarAtString package, DollarAtString type, DollarAtString subtypes, DollarAtString name, DollarAtString signature, - DollarAtString input + DollarAtString input, DollarAtString isVarargsArray where not exists(CharacteristicsImpl::UninterestingToModelCharacteristic u | u.appliesToEndpoint(endpoint) ) and - endpoint = getSampleForSignature(9, package, type, subtypes, name, signature, input) and + endpoint = + getSampleForSignature(9, package, type, subtypes, name, signature, input, isVarargsArray) and // If a node is already a known sink for any of our existing ATM queries and is already modeled as a MaD sink, we // don't include it as a candidate. Otherwise, we might include it as a candidate for query A, but the model will // label it as a sink for one of the sink types of query B, for which it's already a known sink. This would result in // overlap between our detected sinks and the pre-existing modeling. We assume that, if a sink has already been // modeled in a MaD model, then it doesn't belong to any additional sink types, and we don't need to reexamine it. not CharacteristicsImpl::isSink(endpoint, _, _) and - meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input) and + meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input, isVarargsArray) and // The message is the concatenation of all sink types for which this endpoint is known neither to be a sink nor to be // a non-sink, and we surface only endpoints that have at least one such sink type. message = @@ -83,4 +86,5 @@ select endpoint.asNode(), subtypes, "subtypes", // name, "name", // method name signature, "signature", // - input, "input" // + input, "input", // + isVarargsArray, "isVarargsArray" diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql index 4022dbfdeda6..881d39c0b8b1 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql @@ -43,7 +43,8 @@ Endpoint getSampleForCharacteristic(EndpointCharacteristic c, int limit) { from Endpoint endpoint, EndpointCharacteristic characteristic, float confidence, string message, ApplicationModeMetadataExtractor meta, DollarAtString package, DollarAtString type, - DollarAtString subtypes, DollarAtString name, DollarAtString signature, DollarAtString input + DollarAtString subtypes, DollarAtString name, DollarAtString signature, DollarAtString input, + DollarAtString isVarargsArray where endpoint = getSampleForCharacteristic(characteristic, 100) and confidence >= SharedCharacteristics::highConfidence() and @@ -51,7 +52,7 @@ where // Exclude endpoints that have contradictory endpoint characteristics, because we only want examples we're highly // certain about in the prompt. not erroneousEndpoints(endpoint, _, _, _, _, false) and - meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input) and + meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input, isVarargsArray) and // It's valid for a node to satisfy the logic for both `isSink` and `isSanitizer`, but in that case it will be // treated by the actual query as a sanitizer, since the final logic is something like // `isSink(n) and not isSanitizer(n)`. We don't want to include such nodes as negative examples in the prompt, because @@ -71,4 +72,5 @@ select endpoint.asNode(), subtypes, "subtypes", // name, "name", // signature, "signature", // - input, "input" // + input, "input", // + isVarargsArray, "isVarargsArray" // diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql index 99396ce40e26..67da9f362ada 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql @@ -15,12 +15,12 @@ private import AutomodelJavaUtil from Endpoint endpoint, SinkType sinkType, ApplicationModeMetadataExtractor meta, DollarAtString package, DollarAtString type, DollarAtString subtypes, DollarAtString name, - DollarAtString signature, DollarAtString input + DollarAtString signature, DollarAtString input, DollarAtString isVarargsArray where // Exclude endpoints that have contradictory endpoint characteristics, because we only want examples we're highly // certain about in the prompt. not erroneousEndpoints(endpoint, _, _, _, _, false) and - meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input) and + meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input, isVarargsArray) and // Extract positive examples of sinks belonging to the existing ATM query configurations. CharacteristicsImpl::isKnownSink(endpoint, sinkType) and exists(CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext())) @@ -32,4 +32,5 @@ select endpoint.asNode(), subtypes, "subtypes", // name, "name", // signature, "signature", // - input, "input" // + input, "input", // + isVarargsArray, "isVarargsArray" diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected index 71b20780172a..0055a15eafd8 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected @@ -1,3 +1,3 @@ -| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | -| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | -| Test.java:51:3:54:3 | new ..[] { .. } | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:54:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | +| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:51:3:54:3 | new ..[] { .. } | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:54:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected index 9db66bdeed35..1ec3bd7eb887 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected @@ -1,2 +1,2 @@ -| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:46:4:46:5 | f2 | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | -| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:52:4:52:4 | p | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | +| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:46:4:46:5 | f2 | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:52:4:52:4 | p | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected index ded3d5a9f97b..5a6f3992f940 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected @@ -1,3 +1,3 @@ -| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:26:4:26:9 | source | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | -| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:27:4:27:9 | target | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | -| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:34:4:34:11 | openPath | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | +| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:26:4:26:9 | source | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:27:4:27:9 | target | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:34:4:34:11 | openPath | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | From 650ff8db87db09228b96cf44d6edff73d2e1951d Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Mon, 7 Aug 2023 11:08:27 +0200 Subject: [PATCH 3/8] Java: automodel comments --- .../AutomodelApplicationModeCharacteristics.qll | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll index 4133bd5c8f12..241645eeaf2a 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll +++ b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll @@ -33,6 +33,9 @@ newtype TApplicationModeEndpoint = idx = min(Argument arg, int n | arg = call.getArgument(n) and arg.isVararg() | n) } +/** + * An endpoint is a node that is a candidate for modeling. + */ abstract private class ApplicationModeEndpoint extends TApplicationModeEndpoint { abstract predicate isArgOf(Call c, int idx); @@ -82,6 +85,15 @@ class InstanceArgument extends ApplicationModeEndpoint, TInstanceArgument { override string toString() { result = arg.toString() } } +/** + * An endpoint that represents an implicit varargs array. + * We choose to represent the varargs array as a single endpoint, rather than as multiple endpoints. + * + * This avoids the problem of having to deal with redundant endpoints downstream. + * + * In order to be able to distinguish between varargs endpoints and regular endpoints, we export the `isVarargsArray` + * meta data field in the extraction queries. + */ class ImplicitVarargsArray extends ApplicationModeEndpoint, TImplicitVarargsArray { Call call; DataFlow::ImplicitVarargsArray varargs; From e1a5eba61b8558f55f230d6969027fb33acb51b2 Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Mon, 7 Aug 2023 12:18:28 +0200 Subject: [PATCH 4/8] Java: automodel application mode: refactor varargs endpoint class to rely on normal argument node for nicer extracted examples --- ...utomodelApplicationModeCharacteristics.qll | 20 +++++++++++-------- ...lApplicationModeExtractCandidates.expected | 2 +- .../Test.java | 4 ++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll index 241645eeaf2a..fe43bbd133d3 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll +++ b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll @@ -24,13 +24,17 @@ newtype JavaRelatedLocationType = CallContext() newtype TApplicationModeEndpoint = TExplicitArgument(Call call, DataFlow::Node arg) { exists(Argument argExpr | - arg.asExpr() = argExpr and not argExpr.isVararg() and call = argExpr.getCall() + arg.asExpr() = argExpr and call = argExpr.getCall() and not argExpr.isVararg() ) } or TInstanceArgument(Call call, DataFlow::Node arg) { arg = DataFlow::getInstanceArgument(call) } or - TImplicitVarargsArray(Call call, DataFlow::ImplicitVarargsArray varargs, int idx) { - varargs.getCall() = call and - idx = min(Argument arg, int n | arg = call.getArgument(n) and arg.isVararg() | n) + TImplicitVarargsArray(Call call, DataFlow::Node arg, int idx) { + exists(Argument argExpr | + arg.asExpr() = argExpr and + call = argExpr.getCall() and + argExpr.isVararg() and + idx = min(int n | argExpr = call.getArgument(n) and argExpr.isVararg() | n) + ) } /** @@ -96,18 +100,18 @@ class InstanceArgument extends ApplicationModeEndpoint, TInstanceArgument { */ class ImplicitVarargsArray extends ApplicationModeEndpoint, TImplicitVarargsArray { Call call; - DataFlow::ImplicitVarargsArray varargs; + DataFlow::Node vararg; int idx; - ImplicitVarargsArray() { this = TImplicitVarargsArray(call, varargs, idx) } + ImplicitVarargsArray() { this = TImplicitVarargsArray(call, vararg, idx) } override predicate isArgOf(Call c, int i) { c = call and i = idx } override Top asTop() { result = this.getCall() } - override DataFlow::Node asNode() { result = varargs } + override DataFlow::Node asNode() { result = vararg } - override string toString() { result = varargs.toString() } + override string toString() { result = vararg.toString() } } /** diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected index 0055a15eafd8..f411dbabd979 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected @@ -1,3 +1,3 @@ | Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | | Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:51:3:54:3 | new ..[] { .. } | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:54:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | +| Test.java:53:4:53:4 | o | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:54:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java index 8991475134c7..43d8919759ce 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java @@ -47,10 +47,10 @@ public static int compareFiles(File f1, File f2) { ); } - public static void FilesWalkExample(Path p) throws Exception { + public static void FilesWalkExample(Path p, FileVisitOption o) throws Exception { Files.walk( p, // negative example (modeled as a taint step) - FileVisitOption.FOLLOW_LINKS // the implicit varargs array is a candidate + o // the implicit varargs array is a candidate ); } } From 3433437034b489e077eedbc9b4fa6f1452d6398d Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Mon, 7 Aug 2023 12:33:39 +0200 Subject: [PATCH 5/8] Java: automodel application mode: only extract the first argument corresponding to a varargs array --- .../src/Telemetry/AutomodelApplicationModeCharacteristics.qll | 4 ++-- .../AutomodelApplicationModeExtractCandidates.expected | 2 +- .../Telemetry/AutomodelApplicationModeExtraction/Test.java | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll index fe43bbd133d3..11e4195035a6 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll +++ b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll @@ -31,9 +31,9 @@ newtype TApplicationModeEndpoint = TImplicitVarargsArray(Call call, DataFlow::Node arg, int idx) { exists(Argument argExpr | arg.asExpr() = argExpr and - call = argExpr.getCall() and + call.getArgument(idx) = argExpr and argExpr.isVararg() and - idx = min(int n | argExpr = call.getArgument(n) and argExpr.isVararg() | n) + not exists(int i | i < idx and call.getArgument(i).(Argument).isVararg()) ) } diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected index f411dbabd979..776f37ff78e3 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected @@ -1,3 +1,3 @@ | Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | | Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:53:4:53:4 | o | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:54:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | +| Test.java:53:4:53:4 | o | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:56:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java index 43d8919759ce..1b5cea4b9077 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/Test.java @@ -50,7 +50,9 @@ public static int compareFiles(File f1, File f2) { public static void FilesWalkExample(Path p, FileVisitOption o) throws Exception { Files.walk( p, // negative example (modeled as a taint step) - o // the implicit varargs array is a candidate + o, // the implicit varargs array is a candidate + o // not a candidate (only the first arg corresponding to a varargs array + // is extracted) ); } } From a9906f6f7bcf2d9474e57a96c0616c344ba6ac79 Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Fri, 11 Aug 2023 09:15:09 +0200 Subject: [PATCH 6/8] Java: fix - add extra $@ Co-authored-by: Jami <57204504+jcogs33@users.noreply.github.com> --- .../src/Telemetry/AutomodelApplicationModeExtractCandidates.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql index 945d6e1e0327..d58af008d875 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql @@ -80,7 +80,7 @@ where sinkType, ", " ) select endpoint.asNode(), - message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // + message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@, $@.", // CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext()), "CallContext", // package, "package", // type, "type", // From 4107758c8a72eb5cb12310bdcb230953fa075da5 Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Mon, 14 Aug 2023 09:49:50 +0200 Subject: [PATCH 7/8] Java: automodel extraction: add strings to query selection --- .../AutomodelApplicationModeExtractNegativeExamples.ql | 2 +- .../AutomodelApplicationModeExtractPositiveExamples.ql | 2 +- .../AutomodelApplicationModeExtractCandidates.expected | 6 +++--- ...AutomodelApplicationModeExtractNegativeExamples.expected | 4 ++-- ...AutomodelApplicationModeExtractPositiveExamples.expected | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql index 881d39c0b8b1..a1cb9c8961a8 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractNegativeExamples.ql @@ -65,7 +65,7 @@ where ) and message = characteristic select endpoint.asNode(), - message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // + message + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@, $@.", // CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext()), "CallContext", // package, "package", // type, "type", // diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql b/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql index 67da9f362ada..dac9bef07287 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql +++ b/java/ql/src/Telemetry/AutomodelApplicationModeExtractPositiveExamples.ql @@ -25,7 +25,7 @@ where CharacteristicsImpl::isKnownSink(endpoint, sinkType) and exists(CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext())) select endpoint.asNode(), - sinkType + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@.", // + sinkType + "\nrelated locations: $@." + "\nmetadata: $@, $@, $@, $@, $@, $@, $@.", // CharacteristicsImpl::getRelatedLocationOrCandidate(endpoint, CallContext()), "CallContext", // package, "package", // type, "type", // diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected index 776f37ff78e3..a1cdd69b520b 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected @@ -1,3 +1,3 @@ -| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:53:4:53:4 | o | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:51:3:56:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | +| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:53:4:53:4 | o | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:51:3:56:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected index 1ec3bd7eb887..bb94a21f4489 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected @@ -1,2 +1,2 @@ -| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:46:4:46:5 | f2 | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:52:4:52:4 | p | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:46:4:46:5 | f2 | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:52:4:52:4 | p | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected index 5a6f3992f940..6f8a9cadf570 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected @@ -1,3 +1,3 @@ -| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:26:4:26:9 | source | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:27:4:27:9 | target | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@. | Test.java:34:4:34:11 | openPath | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:26:4:26:9 | source | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:27:4:27:9 | target | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:34:4:34:11 | openPath | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | From 1a95a344412a633d219d50e19f78bacfaedf21aa Mon Sep 17 00:00:00 2001 From: Stephan Brandauer Date: Mon, 14 Aug 2023 09:54:44 +0200 Subject: [PATCH 8/8] Java: automodel: use the call for call context, rather than the argument --- .../Telemetry/AutomodelApplicationModeCharacteristics.qll | 2 +- .../AutomodelApplicationModeExtractCandidates.expected | 4 ++-- ...AutomodelApplicationModeExtractNegativeExamples.expected | 4 ++-- ...AutomodelApplicationModeExtractPositiveExamples.expected | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll index 11e4195035a6..90ec5e93f8bb 100644 --- a/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll +++ b/java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll @@ -185,7 +185,7 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig */ RelatedLocation getRelatedLocation(Endpoint e, RelatedLocationType type) { type = CallContext() and - result = e.asTop() + result = e.getCall() } } diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected index a1cdd69b520b..5f946f49d193 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected @@ -1,3 +1,3 @@ -| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:11 | reference | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:10 | supplier | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:16:3:16:11 | reference | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:16:3:16:24 | set(...) | CallContext | file://java.util.concurrent.atomic:1:1:1:1 | java.util.concurrent.atomic | package | file://AtomicReference:1:1:1:1 | AtomicReference | type | file://false:1:1:1:1 | false | subtypes | file://set:1:1:1:1 | set | name | file://(String):1:1:1:1 | (String) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:21:3:21:10 | supplier | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:21:3:21:16 | get(...) | CallContext | file://java.util.function:1:1:1:1 | java.util.function | package | file://Supplier:1:1:1:1 | Supplier | type | file://true:1:1:1:1 | true | subtypes | file://get:1:1:1:1 | get | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://false:1:1:1:1 | false | isVarargsArray | | Test.java:53:4:53:4 | o | command-injection, path-injection, request-forgery, sql-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:51:3:56:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://true:1:1:1:1 | true | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected index bb94a21f4489..1ca68cb2281a 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected @@ -1,2 +1,2 @@ -| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:46:4:46:5 | f2 | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:52:4:52:4 | p | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:46:4:46:5 | f2 | known non-sink\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:45:10:47:3 | compareTo(...) | CallContext | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:52:4:52:4 | p | taint step\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:51:3:56:3 | walk(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | diff --git a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected index 6f8a9cadf570..e07f92f4baf1 100644 --- a/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected +++ b/java/ql/test/query-tests/Telemetry/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractPositiveExamples.expected @@ -1,3 +1,3 @@ -| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:26:4:26:9 | source | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:27:4:27:9 | target | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://false:1:1:1:1 | false | isVarargsArray | -| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:34:4:34:11 | openPath | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:26:4:26:9 | source | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:25:3:29:3 | copy(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:27:4:27:9 | target | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:25:3:29:3 | copy(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://copy:1:1:1:1 | copy | name | file://(Path,Path,CopyOption[]):1:1:1:1 | (Path,Path,CopyOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://false:1:1:1:1 | false | isVarargsArray | +| Test.java:34:4:34:11 | openPath | path-injection\nrelated locations: $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@. | Test.java:33:10:35:3 | newInputStream(...) | CallContext | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://false:1:1:1:1 | false | isVarargsArray |