From a1dc87496a02c1eeb538630fd867c8a189256fd9 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 13:56:38 +0100 Subject: [PATCH 1/7] Shared: Replace a 'count' with a 'strictcount' to prevent a CP when testing on C++. --- .../codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll index 0678d07e7f47..61b6b2434dc0 100644 --- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll +++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll @@ -687,7 +687,7 @@ module MakeModelGenerator< private DataFlow::ParameterNode parameter; ContentDataFlowSummaryTargetApi() { - count(string input, string output | + strictcount(string input, string output | exists( PropagateContentFlow::AccessPath reads, ReturnNodeExt returnNodeExt, PropagateContentFlow::AccessPath stores From 732fcbf1c908abbe7f6976e7aff17712eca8ab89 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 13:58:59 +0100 Subject: [PATCH 2/7] Shared: Move 'asParameter' out of the class signature. --- .../modelgenerator/internal/ModelGeneratorImpl.qll | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll index 61b6b2434dc0..270301f361d3 100644 --- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll +++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll @@ -55,11 +55,8 @@ signature module ModelGeneratorInputSig */ Callable getAsExprEnclosingCallable(); - /** - * Gets the parameter corresponding to this node, if any. - */ - Parameter asParameter(); - } + /** Gets the parameter corresponding to this node, if any. */ + Parameter asParameter(NodeExtended n); /** * A class of callables that are potentially relevant for generating summary or @@ -390,7 +387,7 @@ module MakeModelGenerator< * Gets the MaD string representation of the parameter node `p`. */ string parameterNodeAsInput(DataFlow::ParameterNode p) { - result = parameterAccess(p.(NodeExtended).asParameter()) + result = parameterAccess(asParameter(p)) or result = qualifierString() and p instanceof InstanceParameterNode } @@ -613,7 +610,7 @@ module MakeModelGenerator< * when used in content flow. */ private string parameterNodeAsContentInput(DataFlow::ParameterNode p) { - result = parameterContentAccess(p.(NodeExtended).asParameter()) + result = parameterContentAccess(asParameter(p)) or result = qualifierString() and p instanceof InstanceParameterNode } From c484945f395d31e7c6c131ef373baf467ca8b555 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:00:11 +0100 Subject: [PATCH 3/7] Shared: Move 'getEnclosingCallable' and 'getAsExprEnclosingCallable' out of the class signature. --- .../internal/ModelGeneratorImpl.qll | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll index 270301f361d3..da06e74e10f0 100644 --- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll +++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll @@ -44,16 +44,15 @@ signature module ModelGeneratorInputSig * Gets the type of this node. */ Type getType(); + } - /** - * Gets the enclosing callable of this node. - */ - Callable getEnclosingCallable(); + /** Gets the enclosing callable of `node`. */ + Callable getEnclosingCallable(NodeExtended node); - /** - * Gets the enclosing callable of this node, when considered as an expression. - */ - Callable getAsExprEnclosingCallable(); + /** + * Gets the enclosing callable of `node`, when considered as an expression. + */ + Callable getAsExprEnclosingCallable(NodeExtended node); /** Gets the parameter corresponding to this node, if any. */ Parameter asParameter(NodeExtended n); @@ -462,7 +461,7 @@ module MakeModelGenerator< predicate isSource(DataFlow::Node source, FlowState state) { source instanceof DataFlow::ParameterNode and exists(Callable c | - c = source.(NodeExtended).getEnclosingCallable() and + c = getEnclosingCallable(source) and c instanceof DataFlowSummaryTargetApi and not isUninterestingForHeuristicDataFlowModels(c) ) and @@ -472,7 +471,7 @@ module MakeModelGenerator< predicate isSink(DataFlow::Node sink, FlowState state) { sink instanceof ReturnNodeExt and not isOwnInstanceAccessNode(sink) and - not exists(captureQualifierFlow(sink.(NodeExtended).getAsExprEnclosingCallable())) and + not exists(captureQualifierFlow(getAsExprEnclosingCallable(sink))) and (state instanceof TaintRead or state instanceof TaintStore) } @@ -516,8 +515,8 @@ module MakeModelGenerator< DataFlowSummaryTargetApi api, DataFlow::ParameterNode p, ReturnNodeExt returnNodeExt ) { exists(string input, string output | - p.(NodeExtended).getEnclosingCallable() = api and - returnNodeExt.getEnclosingCallable() = api and + getEnclosingCallable(p) = api and + getEnclosingCallable(returnNodeExt) = api and input = parameterNodeAsInput(p) and output = getOutput(returnNodeExt) and input != output and @@ -567,11 +566,12 @@ module MakeModelGenerator< private module PropagateContentFlowConfig implements ContentDataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof DataFlow::ParameterNode and - source.(NodeExtended).getEnclosingCallable() instanceof DataFlowSummaryTargetApi + getEnclosingCallable(source) instanceof DataFlowSummaryTargetApi } predicate isSink(DataFlow::Node sink) { - sink.(ReturnNodeExt).getEnclosingCallable() instanceof DataFlowSummaryTargetApi + sink instanceof ReturnNodeExt and + getEnclosingCallable(sink) instanceof DataFlowSummaryTargetApi } predicate isAdditionalFlowStep = isAdditionalContentFlowStep/2; @@ -664,8 +664,8 @@ module MakeModelGenerator< PropagateContentFlow::AccessPath stores, boolean preservesValue ) { PropagateContentFlow::flow(p, reads, returnNodeExt, stores, preservesValue) and - returnNodeExt.getEnclosingCallable() = api and - p.(NodeExtended).getEnclosingCallable() = api + getEnclosingCallable(returnNodeExt) = api and + getEnclosingCallable(p) = api } /** @@ -709,8 +709,8 @@ module MakeModelGenerator< PropagateContentFlow::AccessPath stores, boolean preservesValue ) { PropagateContentFlow::flow(p, reads, returnNodeExt, stores, preservesValue) and - returnNodeExt.getEnclosingCallable() = api and - p.(NodeExtended).getEnclosingCallable() = api and + getEnclosingCallable(returnNodeExt) = api and + getEnclosingCallable(p) = api and p = api.getARelevantParameterNode() } @@ -982,7 +982,8 @@ module MakeModelGenerator< } predicate isSink(DataFlow::Node sink) { - sink.(ReturnNodeExt).getEnclosingCallable() instanceof DataFlowSourceTargetApi + sink instanceof ReturnNodeExt and + getEnclosingCallable(sink) instanceof DataFlowSourceTargetApi } DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSinkCallContext } @@ -1005,8 +1006,8 @@ module MakeModelGenerator< exists(NodeExtended source, ReturnNodeExt sink, string kind | PropagateFromSource::flow(source, sink) and sourceNode(source, kind) and - api = sink.getEnclosingCallable() and - not irrelevantSourceSinkApi(source.getEnclosingCallable(), api) and + api = getEnclosingCallable(sink) and + not irrelevantSourceSinkApi(getEnclosingCallable(source), api) and result = ModelPrinting::asSourceModel(api, getOutput(sink), kind) ) } @@ -1021,7 +1022,7 @@ module MakeModelGenerator< module PropagateToSinkConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { apiSource(source) and - source.(NodeExtended).getEnclosingCallable() instanceof DataFlowSinkTargetApi + getEnclosingCallable(source) instanceof DataFlowSinkTargetApi } predicate isSink(DataFlow::Node sink) { @@ -1050,7 +1051,7 @@ module MakeModelGenerator< exists(NodeExtended src, NodeExtended sink, string kind | PropagateToSink::flow(src, sink) and sinkNode(sink, kind) and - api = src.getEnclosingCallable() and + api = getEnclosingCallable(src) and result = ModelPrinting::asSinkModel(api, asInputArgument(src), kind) ) } From 04bf908a4bcb26179988550f84bdf4f4c2cb2635 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:01:00 +0100 Subject: [PATCH 4/7] C#: Fixup MaD input. --- .../utils/modelgenerator/internal/CaptureModels.qll | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll index 125204d7c5bd..ce83369df077 100644 --- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll +++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll @@ -22,10 +22,16 @@ module ModelGeneratorInput implements ModelGeneratorInputSig`. */ From b6c658767e23f0c74d7825bbcf65fd9584948d24 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 10 Apr 2025 14:01:11 +0100 Subject: [PATCH 5/7] Java: Fixup MaD input. --- .../utils/modelgenerator/internal/CaptureModels.qll | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll index 1f96d8ad296d..9dd317b30067 100644 --- a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll +++ b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll @@ -32,10 +32,16 @@ module ModelGeneratorInput implements ModelGeneratorInputSig Date: Thu, 10 Apr 2025 14:01:20 +0100 Subject: [PATCH 6/7] Rust: Fixup MaD input. --- .../utils/modelgenerator/internal/CaptureModels.qll | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll index 3a14f6a4a489..237da46750bd 100644 --- a/rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll +++ b/rust/ql/src/utils/modelgenerator/internal/CaptureModels.qll @@ -20,15 +20,17 @@ module ModelGeneratorInput implements ModelGeneratorInputSig Date: Thu, 10 Apr 2025 14:02:31 +0100 Subject: [PATCH 7/7] Shared: Provide a hook to MaD generation to modify the 'ReturnValue' string. --- .../internal/ModelGeneratorImpl.qll | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll index da06e74e10f0..37934b921147 100644 --- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll +++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll @@ -222,6 +222,14 @@ signature module ModelGeneratorInputSig */ default Lang::ParameterPosition getReturnKindParamPosition(Lang::ReturnKind node) { none() } + /** + * Gets the string that represents the return value corresponding to the + * return kind `kind`. + * + * For most languages this will be the string "ReturnValue". + */ + default string getReturnValueString(Lang::ReturnKind kind) { result = "ReturnValue" } + /** * Holds if it is irrelevant to generate models for `api` based on data flow analysis. * @@ -317,9 +325,11 @@ module MakeModelGenerator< private module PrintReturnNodeExt { string getOutput(ReturnNodeExt node) { - node.getKind() instanceof DataFlow::ValueReturnKind and - not exists(node.getPosition()) and - result = "ReturnValue" + exists(DataFlow::ValueReturnKind valueReturnKind | + valueReturnKind = node.getKind() and + not exists(node.getPosition()) and + result = getReturnValueString(valueReturnKind.getKind()) + ) or exists(DataFlow::ParameterPosition pos | pos = node.getPosition() and