From f1445eb52f117b9eb0df7f470a6f29a1ae673446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Tue, 15 Jul 2025 15:21:37 +0200 Subject: [PATCH] [DIFF-INFORMED] Actions: EnvPathInjection https://github.com/d10c/codeql/blob/d10c/diff-informed-phase-3/actions/ql/src/Security/CWE-077/EnvPathInjectionMedium.ql#L30 https://github.com/d10c/codeql/blob/d10c/diff-informed-phase-3/actions/ql/src/Security/CWE-077/EnvPathInjectionCritical.ql#L37 --- .../security/EnvPathInjectionQuery.qll | 31 +++++++++++++++++++ .../CWE-077/EnvPathInjectionCritical.ql | 10 ++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll b/actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll index 33efc9b1bc8..46c1c4d3200 100644 --- a/actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll +++ b/actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll @@ -72,6 +72,25 @@ class EnvPathInjectionFromMaDSink extends EnvPathInjectionSink { EnvPathInjectionFromMaDSink() { madSink(this, "envpath-injection") } } +/** + * Get the relevant event for a sink in EnvPathInjectionCritical.ql where the source type is "artifact". + */ +Event getRelevantArtifactEventInPrivilegedContext(DataFlow::Node sink) { + inPrivilegedContext(sink.asExpr(), result) and + not exists(ControlCheck check | + check.protects(sink.asExpr(), result, ["untrusted-checkout", "artifact-poisoning"]) + ) and + sink instanceof EnvPathInjectionFromFileReadSink +} + +/** + * Get the relevant event for a sink in EnvPathInjectionCritical.ql where the source type is not "artifact". + */ +Event getRelevantNonArtifactEventInPrivilegedContext(DataFlow::Node sink) { + inPrivilegedContext(sink.asExpr(), result) and + not exists(ControlCheck check | check.protects(sink.asExpr(), result, "code-injection")) +} + /** * A taint-tracking configuration for unsafe user input * that is used to construct and evaluate an environment variable. @@ -108,6 +127,18 @@ private module EnvPathInjectionConfig implements DataFlow::ConfigSig { exists(run.getScript().getAFileReadCommand()) ) } + + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSourceLocation(DataFlow::Node source) { none() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.getLocation() + or + result = getRelevantArtifactEventInPrivilegedContext(sink).getLocation() + or + result = getRelevantNonArtifactEventInPrivilegedContext(sink).getLocation() + } } /** Tracks flow of unsafe user input that is used to construct and evaluate the PATH environment variable. */ diff --git a/actions/ql/src/Security/CWE-077/EnvPathInjectionCritical.ql b/actions/ql/src/Security/CWE-077/EnvPathInjectionCritical.ql index 3e6d63a4604..30bb0fd366f 100644 --- a/actions/ql/src/Security/CWE-077/EnvPathInjectionCritical.ql +++ b/actions/ql/src/Security/CWE-077/EnvPathInjectionCritical.ql @@ -21,18 +21,12 @@ import codeql.actions.security.ControlChecks from EnvPathInjectionFlow::PathNode source, EnvPathInjectionFlow::PathNode sink, Event event where EnvPathInjectionFlow::flowPath(source, sink) and - inPrivilegedContext(sink.getNode().asExpr(), event) and ( not source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and - not exists(ControlCheck check | - check.protects(sink.getNode().asExpr(), event, "code-injection") - ) + event = getRelevantNonArtifactEventInPrivilegedContext(sink.getNode()) or source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and - not exists(ControlCheck check | - check.protects(sink.getNode().asExpr(), event, ["untrusted-checkout", "artifact-poisoning"]) - ) and - sink.getNode() instanceof EnvPathInjectionFromFileReadSink + event = getRelevantArtifactEventInPrivilegedContext(sink.getNode()) ) select sink.getNode(), source, sink, "Potential PATH environment variable injection in $@, which may be controlled by an external user ($@).",