From 8065746a2a22b091e8aa6c8b23c6725a29b08b9d Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Fri, 12 May 2023 19:35:08 +0100 Subject: [PATCH] Add query to find context variables that may not work with default setup --- .../default-setup-environment-variables.ql | 2 + queries/default-setup-event-context.ql | 70 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 queries/default-setup-event-context.ql diff --git a/queries/default-setup-environment-variables.ql b/queries/default-setup-environment-variables.ql index 43874c97c..d4a095cdd 100644 --- a/queries/default-setup-environment-variables.ql +++ b/queries/default-setup-environment-variables.ql @@ -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 = [ diff --git a/queries/default-setup-event-context.ql b/queries/default-setup-event-context.ql new file mode 100644 index 000000000..b5d1ca309 --- /dev/null +++ b/queries/default-setup-event-context.ql @@ -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."