Skip to content

Commit 1fa00f1

Browse files
author
Alvaro Muñoz
committed
Capture the event name rathen than the whole event
1 parent 9a137db commit 1fa00f1

File tree

2 files changed

+28
-26
lines changed

2 files changed

+28
-26
lines changed

ql/lib/codeql/actions/dataflow/FlowSources.qll

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,46 +19,50 @@ abstract class RemoteFlowSource extends SourceNode {
1919
abstract string getSourceType();
2020

2121
/** Gets the event that triggered the source. */
22-
abstract Event getEvent();
22+
abstract string getEventName();
2323

2424
override string getThreatModel() { result = "remote" }
2525
}
2626

27+
/**
28+
* A data flow source of user input from github context.
29+
* eg: github.head_ref
30+
*/
2731
class GitHubCtxSource extends RemoteFlowSource {
2832
string flag;
29-
Event event;
33+
string event;
3034

3135
GitHubCtxSource() {
3236
exists(Expression e, string context, string context_prefix |
3337
this.asExpr() = e and
3438
context = e.getExpression() and
35-
event = e.getEnclosingWorkflow().getATriggerEvent() and
3639
normalizeExpr(context) = "github.head_ref" and
37-
contextTriggerDataModel(event.getName(), context_prefix) and
40+
event = e.getEnclosingWorkflow().getATriggerEvent().getName() and
41+
contextTriggerDataModel(event, context_prefix) and
3842
normalizeExpr(context).matches("%" + context_prefix + "%") and
3943
flag = "branch"
4044
)
4145
}
4246

4347
override string getSourceType() { result = flag }
4448

45-
override Event getEvent() { result = event }
49+
override string getEventName() { result = event }
4650
}
4751

4852
class GitHubEventCtxSource extends RemoteFlowSource {
4953
string flag;
5054
string context;
51-
Event event;
55+
string event;
5256

5357
GitHubEventCtxSource() {
5458
exists(Expression e, string regexp |
5559
this.asExpr() = e and
5660
context = e.getExpression() and
57-
event = e.getATriggerEvent() and
61+
event = e.getATriggerEvent().getName() and
5862
(
5963
// the context is available for the job trigger events
6064
exists(string context_prefix |
61-
contextTriggerDataModel(event.getName(), context_prefix) and
65+
contextTriggerDataModel(event, context_prefix) and
6266
normalizeExpr(context).matches("%" + context_prefix + "%")
6367
)
6468
or
@@ -74,15 +78,15 @@ class GitHubEventCtxSource extends RemoteFlowSource {
7478

7579
string getContext() { result = context }
7680

77-
override Event getEvent() { result = event }
81+
override string getEventName() { result = event }
7882
}
7983

8084
abstract class CommandSource extends RemoteFlowSource {
8185
abstract string getCommand();
8286

8387
abstract Run getEnclosingRun();
8488

85-
override Event getEvent() { result = this.getEnclosingRun().getATriggerEvent() }
89+
override string getEventName() { result = this.getEnclosingRun().getATriggerEvent().getName() }
8690
}
8791

8892
class GitCommandSource extends RemoteFlowSource, CommandSource {
@@ -172,19 +176,19 @@ class GitHubEventPathSource extends RemoteFlowSource, CommandSource {
172176

173177
class GitHubEventJsonSource extends RemoteFlowSource {
174178
string flag;
175-
Event event;
179+
string event;
176180

177181
GitHubEventJsonSource() {
178182
exists(Expression e, string context, string regexp |
179183
this.asExpr() = e and
180184
context = e.getExpression() and
181-
event = e.getEnclosingWorkflow().getATriggerEvent() and
185+
event = e.getEnclosingWorkflow().getATriggerEvent().getName() and
182186
untrustedEventPropertiesDataModel(regexp, _) and
183187
(
184188
// only contexts for the triggering events are considered tainted.
185189
// eg: for `pull_request`, we only consider `github.event.pull_request`
186190
exists(string context_prefix |
187-
contextTriggerDataModel(event.getName(), context_prefix) and
191+
contextTriggerDataModel(event, context_prefix) and
188192
normalizeExpr(context).matches("%" + context_prefix + "%")
189193
) and
190194
normalizeExpr(context).regexpMatch("(?i).*" + wrapJsonRegexp(regexp) + ".*")
@@ -199,7 +203,7 @@ class GitHubEventJsonSource extends RemoteFlowSource {
199203

200204
override string getSourceType() { result = flag }
201205

202-
override Event getEvent() { result = event }
206+
override string getEventName() { result = event }
203207
}
204208

205209
/**
@@ -212,7 +216,7 @@ class MaDSource extends RemoteFlowSource {
212216

213217
override string getSourceType() { result = sourceType }
214218

215-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
219+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
216220
}
217221

218222
abstract class FileSource extends RemoteFlowSource { }
@@ -225,20 +229,18 @@ class ArtifactSource extends RemoteFlowSource, FileSource {
225229

226230
override string getSourceType() { result = "artifact" }
227231

228-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
232+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
229233
}
230234

231235
/**
232236
* A file from an untrusted checkout.
233237
*/
234238
private class CheckoutSource extends RemoteFlowSource, FileSource {
235-
Event event;
236-
237239
CheckoutSource() { this.asExpr() instanceof SimplePRHeadCheckoutStep }
238240

239241
override string getSourceType() { result = "artifact" }
240242

241-
override Event getEvent() { result = event }
243+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
242244
}
243245

244246
/**
@@ -255,7 +257,7 @@ class DornyPathsFilterSource extends RemoteFlowSource {
255257

256258
override string getSourceType() { result = "filename" }
257259

258-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
260+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
259261
}
260262

261263
/**
@@ -278,7 +280,7 @@ class TJActionsChangedFilesSource extends RemoteFlowSource {
278280

279281
override string getSourceType() { result = "filename" }
280282

281-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
283+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
282284
}
283285

284286
/**
@@ -301,7 +303,7 @@ class TJActionsVerifyChangedFilesSource extends RemoteFlowSource {
301303

302304
override string getSourceType() { result = "filename" }
303305

304-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
306+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
305307
}
306308

307309
class Xt0rtedSlashCommandSource extends RemoteFlowSource {
@@ -315,7 +317,7 @@ class Xt0rtedSlashCommandSource extends RemoteFlowSource {
315317

316318
override string getSourceType() { result = "text" }
317319

318-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
320+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
319321
}
320322

321323
class ZenteredIssueFormBodyParserSource extends RemoteFlowSource {
@@ -329,7 +331,7 @@ class ZenteredIssueFormBodyParserSource extends RemoteFlowSource {
329331

330332
override string getSourceType() { result = "text" }
331333

332-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
334+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
333335
}
334336

335337
class OctokitRequestActionSource extends RemoteFlowSource {
@@ -352,5 +354,5 @@ class OctokitRequestActionSource extends RemoteFlowSource {
352354

353355
override string getSourceType() { result = "text" }
354356

355-
override Event getEvent() { result = this.asExpr().getATriggerEvent() }
357+
override string getEventName() { result = this.asExpr().getATriggerEvent().getName() }
356358
}

ql/src/Security/CWE-094/CodeInjectionCritical.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event
2323
where
2424
CodeInjectionFlow::flowPath(source, sink) and
2525
inPrivilegedContext(sink.getNode().asExpr(), event) and
26-
source.getNode().(RemoteFlowSource).getEvent() = event and
26+
source.getNode().(RemoteFlowSource).getEventName() = event.getName() and
2727
not exists(ControlCheck check | check.protects(sink.getNode().asExpr(), event, "code-injection")) and
2828
// exclude cases where the sink is a JS script and the expression uses toJson
2929
not exists(UsesStep script |

0 commit comments

Comments
 (0)