Skip to content

Commit 17f7ba2

Browse files
committed
rewrite the taint-step for join() to a flowsummary
1 parent d2bd70d commit 17f7ba2

File tree

2 files changed

+18
-11
lines changed

2 files changed

+18
-11
lines changed

ruby/ql/lib/codeql/ruby/frameworks/core/Array.qll

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,16 @@ module Array {
11861186
}
11871187
}
11881188

1189+
private class JoinSummary extends SimpleSummarizedCallable {
1190+
JoinSummary() { this = ["join"] }
1191+
1192+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
1193+
input = "Argument[self].Element[any]" and
1194+
output = "ReturnValue" and
1195+
preservesValue = false
1196+
}
1197+
}
1198+
11891199
private class PushSummary extends SimpleSummarizedCallable {
11901200
// `append` is an alias for `push`
11911201
PushSummary() { this = ["push", "append"] }
@@ -1816,6 +1826,8 @@ module Array {
18161826
}
18171827
}
18181828

1829+
private import codeql.ruby.frameworks.core.Kernel
1830+
18191831
/**
18201832
* Holds if there an array element `pred` might taint the array defined by `succ`.
18211833
* This is used for queries where we consider an entire array to be tainted if any of its elements are tainted.
@@ -1829,20 +1841,13 @@ module Array {
18291841
)
18301842
or
18311843
exists(DataFlow::CallNode call |
1832-
call.getMethodName() = "[]" and
1844+
call.asExpr().getExpr() = getAStaticArrayCall("[]")
1845+
or
1846+
call.(Kernel::KernelMethodCall).getMethodName() = "Array"
1847+
|
18331848
succ = call and
18341849
pred = call.getArgument(_)
18351850
)
1836-
or
1837-
exists(DataFlow::CallNode call | call.getMethodName() = "join" |
1838-
pred = call.getReceiver() and
1839-
succ = call
1840-
)
1841-
or
1842-
exists(DataFlow::CallNode call | call.getMethodName() = "Array" |
1843-
pred = call.getArgument(_) and
1844-
succ = call
1845-
)
18461851
}
18471852
}
18481853

ruby/ql/test/library-tests/frameworks/pathname/Pathname.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ pathnameInstances
6565
| file://:0:0:0:0 | parameter self of Pathname;Method[relative_path_from] |
6666
| file://:0:0:0:0 | parameter self of Pathname;Method[sub_ext] |
6767
| file://:0:0:0:0 | parameter self of Pathname;Method[to_path] |
68+
| file://:0:0:0:0 | parameter self of join |
6869
| file://:0:0:0:0 | parameter self of sub |
6970
| file://:0:0:0:0 | parameter self of to_s |
7071
fileSystemAccesses
@@ -141,6 +142,7 @@ fileNameSources
141142
| file://:0:0:0:0 | parameter self of Pathname;Method[relative_path_from] |
142143
| file://:0:0:0:0 | parameter self of Pathname;Method[sub_ext] |
143144
| file://:0:0:0:0 | parameter self of Pathname;Method[to_path] |
145+
| file://:0:0:0:0 | parameter self of join |
144146
| file://:0:0:0:0 | parameter self of sub |
145147
| file://:0:0:0:0 | parameter self of to_s |
146148
fileSystemReadAccesses

0 commit comments

Comments
 (0)