Skip to content

File tree

2 files changed

+33
-8
lines changed

2 files changed

+33
-8
lines changed

actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,25 @@ class EnvPathInjectionFromMaDSink extends EnvPathInjectionSink {
7272
EnvPathInjectionFromMaDSink() { madSink(this, "envpath-injection") }
7373
}
7474

75+
/**
76+
* Get the relevant event for a sink in EnvPathInjectionCritical.ql where the source type is "artifact".
77+
*/
78+
Event getRelevantArtifactEventInPrivilegedContext(DataFlow::Node sink) {
79+
inPrivilegedContext(sink.asExpr(), result) and
80+
not exists(ControlCheck check |
81+
check.protects(sink.asExpr(), result, ["untrusted-checkout", "artifact-poisoning"])
82+
) and
83+
sink instanceof EnvPathInjectionFromFileReadSink
84+
}
85+
86+
/**
87+
* Get the relevant event for a sink in EnvPathInjectionCritical.ql where the source type is not "artifact".
88+
*/
89+
Event getRelevantNonArtifactEventInPrivilegedContext(DataFlow::Node sink) {
90+
inPrivilegedContext(sink.asExpr(), result) and
91+
not exists(ControlCheck check | check.protects(sink.asExpr(), result, "code-injection"))
92+
}
93+
7594
/**
7695
* A taint-tracking configuration for unsafe user input
7796
* that is used to construct and evaluate an environment variable.
@@ -108,6 +127,18 @@ private module EnvPathInjectionConfig implements DataFlow::ConfigSig {
108127
exists(run.getScript().getAFileReadCommand())
109128
)
110129
}
130+
131+
predicate observeDiffInformedIncrementalMode() { any() }
132+
133+
Location getASelectedSourceLocation(DataFlow::Node source) { none() }
134+
135+
Location getASelectedSinkLocation(DataFlow::Node sink) {
136+
result = sink.getLocation()
137+
or
138+
result = getRelevantArtifactEventInPrivilegedContext(sink).getLocation()
139+
or
140+
result = getRelevantNonArtifactEventInPrivilegedContext(sink).getLocation()
141+
}
111142
}
112143

113144
/** Tracks flow of unsafe user input that is used to construct and evaluate the PATH environment variable. */

actions/ql/src/Security/CWE-077/EnvPathInjectionCritical.ql

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,12 @@ import codeql.actions.security.ControlChecks
2121
from EnvPathInjectionFlow::PathNode source, EnvPathInjectionFlow::PathNode sink, Event event
2222
where
2323
EnvPathInjectionFlow::flowPath(source, sink) and
24-
inPrivilegedContext(sink.getNode().asExpr(), event) and
2524
(
2625
not source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
27-
not exists(ControlCheck check |
28-
check.protects(sink.getNode().asExpr(), event, "code-injection")
29-
)
26+
event = getRelevantNonArtifactEventInPrivilegedContext(sink.getNode())
3027
or
3128
source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
32-
not exists(ControlCheck check |
33-
check.protects(sink.getNode().asExpr(), event, ["untrusted-checkout", "artifact-poisoning"])
34-
) and
35-
sink.getNode() instanceof EnvPathInjectionFromFileReadSink
29+
event = getRelevantArtifactEventInPrivilegedContext(sink.getNode())
3630
)
3731
select sink.getNode(), source, sink,
3832
"Potential PATH environment variable injection in $@, which may be controlled by an external user ($@).",

0 commit comments

Comments
 (0)