Skip to content

Commit 8065746

Browse files
committed
Add query to find context variables that may not work with default setup
1 parent abb267d commit 8065746

File tree

2 files changed

+72
-0
lines changed

2 files changed

+72
-0
lines changed

queries/default-setup-environment-variables.ql

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ predicate isSafeForDefaultSetup(string envVar) {
1414
envVar.matches("CODEQL_%") or
1515
envVar.matches("CODESCANNING_%") or
1616
envVar.matches("LGTM_%") or
17+
// We flag up usage of potentially unsafe parts of the GitHub event in `default-setup-event-context.ql`.
18+
envVar = "GITHUB_EVENT_PATH" or
1719
// The following environment variables are known to be safe for use with default setup
1820
envVar =
1921
[
+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* @name Some context properties may not exist in default setup workflows
3+
* @id javascript/codeql-action/default-setup-context-properties
4+
* @kind path-problem
5+
* @severity error
6+
*/
7+
8+
import javascript
9+
import DataFlow::PathGraph
10+
11+
class NotParsedLabel extends DataFlow::FlowLabel {
12+
NotParsedLabel() { this = "not-parsed" }
13+
}
14+
15+
class ParsedLabel extends DataFlow::FlowLabel {
16+
ParsedLabel() { this = "parsed" }
17+
}
18+
19+
class EventContextAccessConfiguration extends DataFlow::Configuration {
20+
EventContextAccessConfiguration() { this = "EventContextAccessConfiguration" }
21+
22+
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
23+
source = NodeJSLib::process().getAPropertyRead("env").getAPropertyRead("GITHUB_EVENT_PATH") and
24+
lbl instanceof NotParsedLabel
25+
}
26+
27+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel lbl) {
28+
sink instanceof DataFlow::PropRead and lbl instanceof ParsedLabel
29+
}
30+
31+
override predicate isAdditionalFlowStep(
32+
DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
33+
) {
34+
src = trg.(FileSystemReadAccess).getAPathArgument() and inlbl = outlbl
35+
or
36+
exists(JsonParserCall c |
37+
src = c.getInput() and
38+
trg = c.getOutput() and
39+
inlbl instanceof NotParsedLabel and
40+
outlbl instanceof ParsedLabel
41+
)
42+
or
43+
(
44+
TaintTracking::sharedTaintStep(src, trg) or
45+
DataFlow::SharedFlowStep::step(src, trg) or
46+
DataFlow::SharedFlowStep::step(src, trg, _, _)
47+
) and
48+
inlbl = outlbl
49+
}
50+
}
51+
52+
predicate deepPropertyRead(DataFlow::PropRead originalRead, DataFlow::PropRead read, int depth) {
53+
read = originalRead and depth = 1
54+
or
55+
exists(DataFlow::PropRead prevRead, int prevDepth |
56+
deepPropertyRead(originalRead, prevRead, prevDepth) and
57+
read = prevRead.getAPropertyRead() and
58+
depth = prevDepth + 1
59+
)
60+
}
61+
62+
from EventContextAccessConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
63+
where
64+
cfg.hasFlowPath(source, sink) and
65+
not sink.getNode().asExpr().getFile().getBaseName().matches("%.test.ts")
66+
select sink.getNode(), source, sink,
67+
"This context property may not exist in default setup workflows. If all uses are safe, add it to the list of "
68+
+ "context properties that are known to be safe in " +
69+
"'queries/default-setup-event-context.ql'. If this use is safe but others are not, " +
70+
"dismiss this alert as a false positive."

0 commit comments

Comments
 (0)