From fb841ea5915eefbc98a5ad707f8c64fd0249cf33 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 3 Dec 2025 11:27:35 +0000 Subject: [PATCH] Make predicates containing query logic more self-contained --- .../actions/security/CodeInjectionQuery.qll | 30 +++++++++++-------- .../Security/CWE-094/CodeInjectionCritical.ql | 4 +-- .../Security/CWE-094/CodeInjectionMedium.ql | 4 +-- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll b/actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll index 8bffbbde5d1..bdea8e81962 100644 --- a/actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll +++ b/actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll @@ -93,23 +93,29 @@ private module CodeInjectionConfig implements DataFlow::ConfigSig { module CodeInjectionFlow = TaintTracking::Global; /** - * Holds if the flow from `source` to `sink` has critical severity and they are - * linked by `event`. + * Holds if there is a code injection flow from `source` to `sink` with + * critical severity, linked by `event`. */ -pragma[inline] -predicate criticalSeverity(DataFlow::Node source, DataFlow::Node sink, Event event) { - event = getRelevantCriticalEventForSink(sink) and - source.(RemoteFlowSource).getEventName() = event.getName() +predicate criticalSeverityCodeInjection( + CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event +) { + CodeInjectionFlow::flowPath(source, sink) and + event = getRelevantCriticalEventForSink(sink.getNode()) and + source.getNode().(RemoteFlowSource).getEventName() = event.getName() } -/** Holds if the flow from `source` to `sink` has medium severity. */ -pragma[inline] -predicate mediumSeverity(DataFlow::Node source, DataFlow::Node sink) { - not criticalSeverity(source, sink, _) and +/** + * Holds if there is a code injection flow from `source` to `sink` with medium severity. + */ +predicate mediumSeverityCodeInjection( + CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink +) { + CodeInjectionFlow::flowPath(source, sink) and + not criticalSeverityCodeInjection(source, sink, _) and // exclude cases where the sink is a JS script and the expression uses toJson not exists(UsesStep script | script.getCallee() = "actions/github-script" and - script.getArgumentExpr("script") = sink.asExpr() and - exists(getAToJsonReferenceExpression(sink.asExpr().(Expression).getExpression(), _)) + script.getArgumentExpr("script") = sink.getNode().asExpr() and + exists(getAToJsonReferenceExpression(sink.getNode().asExpr().(Expression).getExpression(), _)) ) } diff --git a/actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql b/actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql index b38332798c2..001aadd66cb 100644 --- a/actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql +++ b/actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql @@ -20,9 +20,7 @@ import CodeInjectionFlow::PathGraph import codeql.actions.security.ControlChecks from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event -where - CodeInjectionFlow::flowPath(source, sink) and - criticalSeverity(source.getNode(), sink.getNode(), event) +where criticalSeverityCodeInjection(source, sink, event) select sink.getNode(), source, sink, "Potential code injection in $@, which may be controlled by an external user ($@).", sink, sink.getNode().asExpr().(Expression).getRawExpression(), event, event.getName() diff --git a/actions/ql/src/Security/CWE-094/CodeInjectionMedium.ql b/actions/ql/src/Security/CWE-094/CodeInjectionMedium.ql index 03bbeb962ce..8bc3fe8f51a 100644 --- a/actions/ql/src/Security/CWE-094/CodeInjectionMedium.ql +++ b/actions/ql/src/Security/CWE-094/CodeInjectionMedium.ql @@ -19,9 +19,7 @@ import codeql.actions.security.CodeInjectionQuery import CodeInjectionFlow::PathGraph from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink -where - CodeInjectionFlow::flowPath(source, sink) and - mediumSeverity(source.getNode(), sink.getNode()) +where mediumSeverityCodeInjection(source, sink) select sink.getNode(), source, sink, "Potential code injection in $@, which may be controlled by an external user.", sink, sink.getNode().asExpr().(Expression).getRawExpression()