Skip to content

Commit

Permalink
Add query to find context variables that may not work with default setup
Browse files Browse the repository at this point in the history
  • Loading branch information
Henry Mercer committed May 12, 2023
1 parent abb267d commit 8065746
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 0 deletions.
2 changes: 2 additions & 0 deletions queries/default-setup-environment-variables.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ predicate isSafeForDefaultSetup(string envVar) {
envVar.matches("CODEQL_%") or
envVar.matches("CODESCANNING_%") or
envVar.matches("LGTM_%") or
// We flag up usage of potentially unsafe parts of the GitHub event in `default-setup-event-context.ql`.
envVar = "GITHUB_EVENT_PATH" or
// The following environment variables are known to be safe for use with default setup
envVar =
[
Expand Down
70 changes: 70 additions & 0 deletions queries/default-setup-event-context.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* @name Some context properties may not exist in default setup workflows
* @id javascript/codeql-action/default-setup-context-properties
* @kind path-problem
* @severity error
*/

import javascript
import DataFlow::PathGraph

class NotParsedLabel extends DataFlow::FlowLabel {
NotParsedLabel() { this = "not-parsed" }
}

class ParsedLabel extends DataFlow::FlowLabel {
ParsedLabel() { this = "parsed" }
}

class EventContextAccessConfiguration extends DataFlow::Configuration {
EventContextAccessConfiguration() { this = "EventContextAccessConfiguration" }

override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
source = NodeJSLib::process().getAPropertyRead("env").getAPropertyRead("GITHUB_EVENT_PATH") and
lbl instanceof NotParsedLabel
}

override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel lbl) {
sink instanceof DataFlow::PropRead and lbl instanceof ParsedLabel
}

override predicate isAdditionalFlowStep(
DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
) {
src = trg.(FileSystemReadAccess).getAPathArgument() and inlbl = outlbl
or
exists(JsonParserCall c |
src = c.getInput() and
trg = c.getOutput() and
inlbl instanceof NotParsedLabel and
outlbl instanceof ParsedLabel
)
or
(
TaintTracking::sharedTaintStep(src, trg) or
DataFlow::SharedFlowStep::step(src, trg) or
DataFlow::SharedFlowStep::step(src, trg, _, _)
) and
inlbl = outlbl
}
}

predicate deepPropertyRead(DataFlow::PropRead originalRead, DataFlow::PropRead read, int depth) {
read = originalRead and depth = 1
or
exists(DataFlow::PropRead prevRead, int prevDepth |
deepPropertyRead(originalRead, prevRead, prevDepth) and
read = prevRead.getAPropertyRead() and
depth = prevDepth + 1
)
}

from EventContextAccessConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where
cfg.hasFlowPath(source, sink) and
not sink.getNode().asExpr().getFile().getBaseName().matches("%.test.ts")
select sink.getNode(), source, sink,
"This context property may not exist in default setup workflows. If all uses are safe, add it to the list of "
+ "context properties that are known to be safe in " +
"'queries/default-setup-event-context.ql'. If this use is safe but others are not, " +
"dismiss this alert as a false positive."

0 comments on commit 8065746

Please sign in to comment.