From 126d24a5226b5b74cd932900ccbc014785d48e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Tue, 15 Jul 2025 15:23:10 +0200 Subject: [PATCH] [DIFF-INFORMED] Actions: EnvVarInjection https://github.com/d10c/codeql/blob/d10c/diff-informed-phase-3/actions/ql/src/Security/CWE-077/EnvVarInjectionMedium.ql#L35 https://github.com/d10c/codeql/blob/d10c/diff-informed-phase-3/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql#L46 --- .../actions/security/EnvVarInjectionQuery.qll | 38 +++++++++++++++++++ .../CWE-077/EnvVarInjectionCritical.ql | 15 +------- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll b/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll index 656ea1207b5..2022e3dca99 100644 --- a/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll +++ b/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll @@ -126,6 +126,32 @@ class EnvVarInjectionFromMaDSink extends EnvVarInjectionSink { EnvVarInjectionFromMaDSink() { madSink(this, "envvar-injection") } } +/** + * Get the relevant event for a sink in EnvVarInjectionCritical.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, + ["envvar-injection", "untrusted-checkout", "artifact-poisoning"]) + ) and + ( + sink instanceof EnvVarInjectionFromFileReadSink or + madSink(sink, "envvar-injection") + ) +} + +/** + * Get the relevant event for a sink in EnvVarInjectionCritical.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, ["envvar-injection", "code-injection"]) + ) +} + /** * A taint-tracking configuration for unsafe user input * that is used to construct and evaluate an environment variable. @@ -163,6 +189,18 @@ private module EnvVarInjectionConfig 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 an environment variable. */ diff --git a/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql b/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql index 28ad3b5b5d2..6f0d9729d6d 100644 --- a/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql +++ b/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql @@ -22,26 +22,15 @@ import codeql.actions.security.ControlChecks from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink, Event event where EnvVarInjectionFlow::flowPath(source, sink) and - inPrivilegedContext(sink.getNode().asExpr(), event) and // exclude paths to file read sinks from non-artifact sources ( // source is text not source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and - not exists(ControlCheck check | - check.protects(sink.getNode().asExpr(), event, ["envvar-injection", "code-injection"]) - ) + event = getRelevantNonArtifactEventInPrivilegedContext(sink.getNode()) or // source is an artifact or a file from an untrusted checkout source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and - not exists(ControlCheck check | - check - .protects(sink.getNode().asExpr(), event, - ["envvar-injection", "untrusted-checkout", "artifact-poisoning"]) - ) and - ( - sink.getNode() instanceof EnvVarInjectionFromFileReadSink or - madSink(sink.getNode(), "envvar-injection") - ) + event = getRelevantArtifactEventInPrivilegedContext(sink.getNode()) ) select sink.getNode(), source, sink, "Potential environment variable injection in $@, which may be controlled by an external user ($@).",