Skip to content

Commit 54d103f

Browse files
author
Alvaro Muñoz
authored
Merge pull request #28 from github/feat/matrix_expressions
Resolve Matrix expression to their possible values
2 parents b2d7c82 + cee0389 commit 54d103f

File tree

5 files changed

+197
-51
lines changed

5 files changed

+197
-51
lines changed

‎ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 80 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -544,9 +544,15 @@ class StrategyImpl extends AstNodeImpl, TStrategyNode {
544544

545545
override YamlMapping getNode() { result = n }
546546

547-
/** Gets a specific matric expression (YamlMapping) by name. */
548-
ExpressionImpl getMatrixVarExpr(string name) {
549-
n.lookup("matrix").(YamlMapping).lookup(name) = result.getNode()
547+
YamlMapping getMatrix() { result = n.lookup("matrix") }
548+
549+
/** Gets a specific matrix expression (YamlMapping) by name. */
550+
ExpressionImpl getMatrixVarExpr(string accessPath) {
551+
exists(MatrixAccessPathImpl p, ScalarValueImpl v |
552+
p.toString() = accessPath and
553+
resolveMatrixAccessPath(n.lookup("matrix"), p).getNode(_) = v.getNode() and
554+
result.getParentNode() = v
555+
)
550556
}
551557

552558
/** Gets a specific matric expression (YamlMapping) by name. */
@@ -777,14 +783,27 @@ class JobImpl extends AstNodeImpl, TJobNode {
777783

778784
/** Gets the runs-on field of the job. */
779785
string getARunsOnLabel() {
780-
exists(string lbl, YamlNode r |
786+
exists(ScalarValueImpl lbl |
781787
(
782-
r = runson.getNode(lbl) and
783-
not lbl = ["group", "labels"]
788+
lbl.getNode() = runson.getNode(_) and
789+
not lbl.getNode() = runson.getNode("group")
784790
or
785-
r = runson.getNode("labels").(YamlMappingLikeNode).getNode(lbl)
791+
lbl.getNode() = runson.getNode("labels").(YamlMappingLikeNode).getNode(_)
786792
) and
787-
result = lbl.trim().regexpReplaceAll("^('|\")", "").regexpReplaceAll("('|\")$", "").trim()
793+
(
794+
not exists(MatrixExpressionImpl e | e.getParentNode() = lbl) and
795+
result =
796+
lbl.getValue()
797+
.trim()
798+
.regexpReplaceAll("^('|\")", "")
799+
.regexpReplaceAll("('|\")$", "")
800+
.trim()
801+
or
802+
exists(MatrixExpressionImpl e |
803+
e.getParentNode() = lbl and
804+
result = e.getLiteralValues()
805+
)
806+
)
788807
)
789808
}
790809
}
@@ -1050,7 +1069,7 @@ private string jobsCtxRegex() {
10501069

10511070
private string envCtxRegex() { result = Utils::wrapRegexp("env\\.([A-Za-z0-9_-]+)") }
10521071

1053-
private string matrixCtxRegex() { result = Utils::wrapRegexp("matrix\\.([A-Za-z0-9_-]+)") }
1072+
private string matrixCtxRegex() { result = Utils::wrapRegexp("matrix\\.(.+)") }
10541073

10551074
private string inputsCtxRegex() {
10561075
result =
@@ -1224,24 +1243,65 @@ class EnvExpressionImpl extends SimpleReferenceExpressionImpl {
12241243
* e.g. `${{ matrix.foo }}`
12251244
*/
12261245
class MatrixExpressionImpl extends SimpleReferenceExpressionImpl {
1227-
string fieldName;
1246+
string fieldAccess;
12281247

12291248
MatrixExpressionImpl() {
12301249
Utils::normalizeExpr(expression).regexpMatch(matrixCtxRegex()) and
1231-
fieldName = Utils::normalizeExpr(expression).regexpCapture(matrixCtxRegex(), 1)
1250+
fieldAccess = Utils::normalizeExpr(expression).regexpCapture(matrixCtxRegex(), 1)
12321251
}
12331252

1234-
override string getFieldName() { result = fieldName }
1253+
override string getFieldName() { result = fieldAccess }
12351254

12361255
override AstNodeImpl getTarget() {
1237-
exists(WorkflowImpl w |
1238-
w.getStrategy().getMatrixVarExpr(fieldName) = result and
1239-
w.getAChildNode*() = this
1240-
)
1241-
or
1242-
exists(JobImpl j |
1243-
j.getStrategy().getMatrixVarExpr(fieldName) = result and
1244-
j.getAChildNode*() = this
1256+
result = this.getEnclosingWorkflow().getStrategy().getMatrixVarExpr(fieldAccess) or
1257+
result = this.getEnclosingJob().getStrategy().getMatrixVarExpr(fieldAccess)
1258+
}
1259+
1260+
string getLiteralValues() {
1261+
exists(StrategyImpl s, MatrixAccessPathImpl p, ScalarValueImpl v |
1262+
(s = this.getEnclosingJob().getStrategy() or s = this.getEnclosingWorkflow().getStrategy()) and
1263+
p.toString() = fieldAccess and
1264+
resolveMatrixAccessPath(s.getMatrix(), p).getNode(_) = v.getNode() and
1265+
// Exclude values containing matrix expressions to avoid recursion
1266+
not exists(MatrixExpressionImpl e | e.getParentNode() = v) and
1267+
result = v.getValue()
12451268
)
12461269
}
12471270
}
1271+
1272+
bindingset[accessPath]
1273+
string explodeAccessPath(string accessPath) {
1274+
result = accessPath or
1275+
result = accessPath.suffix(accessPath.indexOf(".") + 1) or
1276+
result = accessPath.prefix(accessPath.indexOf("."))
1277+
}
1278+
1279+
private newtype TAccessPath =
1280+
TMatrixAccessPathNode(string accessPath) {
1281+
exists(MatrixExpressionImpl e | accessPath = explodeAccessPath(e.getFieldName()))
1282+
}
1283+
1284+
class MatrixAccessPathImpl extends TMatrixAccessPathNode {
1285+
string accessPath;
1286+
1287+
MatrixAccessPathImpl() { this = TMatrixAccessPathNode(accessPath) }
1288+
1289+
string toString() { result = accessPath }
1290+
}
1291+
1292+
private YamlMappingLikeNode resolveMatrixAccessPath(
1293+
YamlMappingLikeNode root, MatrixAccessPathImpl accessPath
1294+
) {
1295+
// access path contains no dots. eg: "os"
1296+
result = root.getNode(accessPath.toString())
1297+
or
1298+
// access path contains dots. eg: "plaform.os"
1299+
exists(MatrixAccessPathImpl first, MatrixAccessPathImpl rest, YamlMappingLikeNode newRoot |
1300+
first.toString() = accessPath.toString().splitAt(".", 0) and
1301+
rest.toString() = accessPath.toString().suffix(first.toString().length() + 1) and
1302+
newRoot = root.getNode(first.toString()) and
1303+
if newRoot instanceof YamlSequence
1304+
then result = resolveMatrixAccessPath(newRoot.(YamlSequence).getElementNode(_), rest)
1305+
else result = resolveMatrixAccessPath(newRoot, rest)
1306+
)
1307+
}

‎ql/lib/codeql/actions/security/SelfHostedQuery.qll

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import actions
2+
import codeql.actions.dataflow.ExternalFlow
3+
4+
bindingset[runner]
5+
predicate isGithubHostedRunner(string runner) {
6+
// list of github hosted repos: https://github.com/actions/runner-images/blob/main/README.md#available-images
7+
runner
8+
.toLowerCase()
9+
.regexpMatch("^(ubuntu-([0-9.]+|latest)|macos-([0-9]+|latest)(-x?large)?|windows-([0-9.]+|latest))$")
10+
}
11+
12+
bindingset[runner]
13+
predicate is3rdPartyHostedRunner(string runner) {
14+
runner.toLowerCase().regexpMatch("^(buildjet|warp)-[a-z0-9-]+$")
15+
}
16+
17+
/**
18+
* This predicate uses data available in the workflow file to identify self-hosted runners.
19+
* It does not know if the repository is public or private.
20+
* It is a best-effort approach to identify self-hosted runners.
21+
*/
22+
predicate staticallyIdentifiedSelfHostedRunner(Job job) {
23+
exists(string label |
24+
job.getATriggerEvent().getName() =
25+
[
26+
"issue_comment", "pull_request", "pull_request_review", "pull_request_review_comment",
27+
"pull_request_target", "workflow_run"
28+
] and
29+
label = job.getARunsOnLabel() and
30+
not isGithubHostedRunner(label) and
31+
not is3rdPartyHostedRunner(label)
32+
)
33+
}
34+
35+
/**
36+
* This predicate uses data available in the job log files to identify self-hosted runners.
37+
* It is a best-effort approach to identify self-hosted runners.
38+
*/
39+
predicate dynamicallyIdentifiedSelfHostedRunner(Job job) {
40+
exists(string runner_info |
41+
workflowDataModel(job.getEnclosingWorkflow().getLocation().getFile().getRelativePath(),
42+
"public", job.getId(), _, _, runner_info) and
43+
runner_info.indexOf("self-hosted:true") > 0
44+
)
45+
}

‎ql/src/Security/CWE-284/CodeExecutionOnSelfHostedRunner.ql

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,36 +11,7 @@
1111
* external/cwe/cwe-284
1212
*/
1313

14-
import actions
15-
import codeql.actions.dataflow.ExternalFlow
16-
17-
/**
18-
* This predicate uses data available in the workflow file to identify self-hosted runners.
19-
* It does not know if the repository is public or private.
20-
* It is a best-effort approach to identify self-hosted runners.
21-
*/
22-
predicate staticallyIdentifiedSelfHostedRunner(Job job) {
23-
exists(string label |
24-
job.getEnclosingWorkflow().getATriggerEvent().getName() =
25-
["pull_request", "pull_request_review", "pull_request_review_comment", "pull_request_target"] and
26-
label = job.getARunsOnLabel() and
27-
// source: https://github.com/boostsecurityio/poutine/blob/main/opa/rego/poutine/utils.rego#L49C3-L49C136
28-
not label
29-
.regexpMatch("(?i)^((ubuntu-(([0-9]{2})\\.04|latest)|macos-([0-9]{2}|latest)(-x?large)?|windows-(20[0-9]{2}|latest)|(buildjet|warp)-[a-z0-9-]+))$")
30-
)
31-
}
32-
33-
/**
34-
* This predicate uses data available in the job log files to identify self-hosted runners.
35-
* It is a best-effort approach to identify self-hosted runners.
36-
*/
37-
predicate dynamicallyIdentifiedSelfHostedRunner(Job job) {
38-
exists(string runner_info |
39-
workflowDataModel(job.getEnclosingWorkflow().getLocation().getFile().getRelativePath(),
40-
"public", job.getId(), _, _, runner_info) and
41-
runner_info.matches("self-hosted:true")
42-
)
43-
}
14+
import codeql.actions.security.SelfHostedQuery
4415

4516
from Job job
4617
where staticallyIdentifiedSelfHostedRunner(job) or dynamicallyIdentifiedSelfHostedRunner(job)

‎ql/test/query-tests/Security/CWE-284/.github/workflows/test1.yml

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,69 @@ jobs:
2626
runs-on: self-hosted-azure
2727
steps:
2828
- run: cmd
29+
test5:
30+
strategy:
31+
fail-fast: false
32+
matrix:
33+
platform:
34+
- name: Linux
35+
os: ubuntu-latest
36+
shell: bash
37+
- name: macOS
38+
os: macos-latest
39+
shell: bash
40+
- name: Windows
41+
os: windows-latest
42+
shell: cmd
43+
node-version:
44+
- 16.14.0
45+
- 16.x
46+
- 18.0.0
47+
- 18.x
48+
- 20.x
49+
runs-on: ${{ matrix.platform.os }}
50+
steps:
51+
- run: cmd
52+
test6:
53+
strategy:
54+
matrix:
55+
os: [ubuntu-latest, macos-latest]
56+
runs-on: ${{ matrix.os }}
57+
steps:
58+
- run: cmd
59+
test7:
60+
strategy:
61+
matrix:
62+
os: [self-hosted, ubuntu-latest]
63+
runs-on: ${{ matrix.os }}
64+
steps:
65+
- run: cmd
66+
test8:
67+
strategy:
68+
matrix:
69+
settings:
70+
- host:
71+
- 'self-hosted'
72+
- 'macos'
73+
- 'arm64'
74+
target: 'x86_64-apple-darwin'
75+
runs-on: ${{ matrix.settings.host }}
76+
steps:
77+
- run: cmd
78+
test9:
79+
strategy:
80+
matrix:
81+
os: ${{ github.repository }}
82+
runs-on: ${{ matrix.os }}
83+
steps:
84+
- run: cmd
85+
test10:
86+
strategy:
87+
matrix:
88+
os: ${{ github.repository }}
89+
foo:
90+
- bar: ${{ github.repository }}
91+
baz: "asdf"
92+
runs-on: ${{ matrix.foo.bar }}
93+
steps:
94+
- run: cmd

‎ql/test/query-tests/Security/CWE-284/CodeExecutionOnSelfHostedRunner.expected

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
| .github/workflows/test1.yml:8:5:11:2 | Job: test1 | Job runs on self-hosted runner |
22
| .github/workflows/test1.yml:12:5:17:2 | Job: test2 | Job runs on self-hosted runner |
33
| .github/workflows/test1.yml:18:5:25:2 | Job: test3 | Job runs on self-hosted runner |
4-
| .github/workflows/test1.yml:26:5:28:15 | Job: test4 | Job runs on self-hosted runner |
4+
| .github/workflows/test1.yml:26:5:29:2 | Job: test4 | Job runs on self-hosted runner |
5+
| .github/workflows/test1.yml:60:5:66:2 | Job: test7 | Job runs on self-hosted runner |
6+
| .github/workflows/test1.yml:67:5:78:2 | Job: test8 | Job runs on self-hosted runner |
7+
| .github/workflows/test1.yml:79:5:85:2 | Job: test9 | Job runs on self-hosted runner |
8+
| .github/workflows/test1.yml:86:5:94:15 | Job: test10 | Job runs on self-hosted runner |

0 commit comments

Comments
 (0)