hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Ruby: WIP: Make Argument[any] and any-named work

Browse Source 
It's not fully working I think the problem is that the code below ties
up `Argument[x]` with parameter positions, and `Parameter[x]` with
argument positions. This flip might be correct for flow-summaries, but
it does NOT seem to be correct for the `path` component  in MaD.

Specifically, quick-eval for ParameterPosition does NOT include `keyword key` while
quick-eval for ArgumentPosition DOES include `keyword key`!

For the test `Foo.sinkAnyNamedArg(key: tainted) # $ MISSING: hasValueFlow=tainted`

c8be8d30b3/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsSpecific.qll (L130-L133)
This commit is contained in:
Rasmus Wriedt Larsen
2022-05-19 15:31:29 +02:00
parent df83a51e1e
commit 7784b9f879
5 changed files with 44 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
ruby/ql/lib/codeql/ruby/ApiGraphs.qll
View File

@@ -796,6 +796,20 @@ module API {
or
pos.isBlock() and
result = Label::blockParameter()
or
pos.isAny() and
(
result = Label::parameter(_)
or
result = Label::keywordParameter(_)
or
result = Label::blockParameter()
// NOTE: `self` should NOT be included, as described in the QLDoc for `isAny()`
)
// TODO: needs handling of `self` ParameterPosition
// or
// pos.isSelf() and
// ...
}
}
}

6
ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImplSpecific.qll
View File

@@ -281,4 +281,10 @@ ParameterPosition parseArgBody(string s) {
or
s = "block" and
result.isBlock()
or
s = "any" and
result.isAny()
or
s = "any-named" and
result.isKeyword(_)
}

2
ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModelsSpecific.qll
View File

@@ -181,7 +181,7 @@ predicate isExtraValidTokenArgumentInIdentifyingAccessPath(string name, string a
or
name = ["Argument", "Parameter"] and
(
argument = ["self", "block"]
argument = ["self", "block", "any", "any-named"]
or
argument.regexpMatch("\\w+:") // keyword argument
)

22
ruby/ql/test/library-tests/dataflow/summaries/Summaries.expected
View File

@@ -21,10 +21,15 @@ edges
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:51:24:51:30 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:54:22:54:28 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:55:17:55:23 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:57:27:57:33 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:61:32:61:38 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:63:23:63:29 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:93:16:93:22 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:99:14:99:20 | tainted : |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:102:16:102:22 | tainted |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:102:16:102:22 | tainted |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:103:21:103:27 | tainted |
| summaries.rb:1:11:1:36 | call to identity : | summaries.rb:103:21:103:27 | tainted |
| summaries.rb:1:20:1:36 | call to source : | summaries.rb:1:11:1:36 | call to identity : |
| summaries.rb:1:20:1:36 | call to source : | summaries.rb:1:11:1:36 | call to identity : |
| summaries.rb:4:12:7:3 | call to apply_block : | summaries.rb:9:6:9:13 | tainted2 |
@@ -59,6 +64,7 @@ edges
| summaries.rb:51:24:51:30 | tainted : | summaries.rb:51:6:51:31 | call to namedArg |
| summaries.rb:54:22:54:28 | tainted : | summaries.rb:54:6:54:29 | call to anyArg |
| summaries.rb:55:17:55:23 | tainted : | summaries.rb:55:6:55:24 | call to anyArg |
| summaries.rb:57:27:57:33 | tainted : | summaries.rb:57:6:57:34 | call to anyNamedArg |
| summaries.rb:61:32:61:38 | tainted : | summaries.rb:61:6:61:39 | call to anyPositionFromOne |
| summaries.rb:63:23:63:29 | tainted : | summaries.rb:63:40:63:40 | x : |
| summaries.rb:63:40:63:40 | x : | summaries.rb:64:8:64:8 | x |
@@ -89,6 +95,8 @@ edges
| summaries.rb:88:6:88:6 | a [element 2] : | summaries.rb:88:6:88:9 | ...[...] |
| summaries.rb:88:6:88:6 | a [element 2] : | summaries.rb:88:6:88:9 | ...[...] |
| summaries.rb:93:16:93:22 | [post] tainted : | summaries.rb:99:14:99:20 | tainted : |
| summaries.rb:93:16:93:22 | [post] tainted : | summaries.rb:102:16:102:22 | tainted |
| summaries.rb:93:16:93:22 | [post] tainted : | summaries.rb:103:21:103:27 | tainted |
| summaries.rb:93:16:93:22 | tainted : | summaries.rb:93:16:93:22 | [post] tainted : |
| summaries.rb:93:16:93:22 | tainted : | summaries.rb:93:25:93:25 | [post] y : |
| summaries.rb:93:16:93:22 | tainted : | summaries.rb:93:33:93:33 | [post] z : |
@@ -156,6 +164,8 @@ nodes
| summaries.rb:54:22:54:28 | tainted : | semmle.label | tainted : |
| summaries.rb:55:6:55:24 | call to anyArg | semmle.label | call to anyArg |
| summaries.rb:55:17:55:23 | tainted : | semmle.label | tainted : |
| summaries.rb:57:6:57:34 | call to anyNamedArg | semmle.label | call to anyNamedArg |
| summaries.rb:57:27:57:33 | tainted : | semmle.label | tainted : |
| summaries.rb:61:6:61:39 | call to anyPositionFromOne | semmle.label | call to anyPositionFromOne |
| summaries.rb:61:32:61:38 | tainted : | semmle.label | tainted : |
| summaries.rb:63:23:63:29 | tainted : | semmle.label | tainted : |
@@ -202,9 +212,12 @@ nodes
| summaries.rb:99:1:99:1 | [post] x : | semmle.label | [post] x : |
| summaries.rb:99:14:99:20 | tainted : | semmle.label | tainted : |
| summaries.rb:100:6:100:6 | x | semmle.label | x |
| summaries.rb:102:16:102:22 | tainted | semmle.label | tainted |
| summaries.rb:102:16:102:22 | tainted | semmle.label | tainted |
| summaries.rb:103:21:103:27 | tainted | semmle.label | tainted |
| summaries.rb:103:21:103:27 | tainted | semmle.label | tainted |
subpaths
invalidSpecComponent
| ;;Member[Foo].Method[anyNamedArg] | Argument[any-named] | Argument[any-named] |
#select
| summaries.rb:2:6:2:12 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:2:6:2:12 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:2:6:2:12 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:2:6:2:12 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
@@ -235,6 +248,7 @@ invalidSpecComponent
| summaries.rb:51:6:51:31 | call to namedArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:51:6:51:31 | call to namedArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:54:6:54:29 | call to anyArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:54:6:54:29 | call to anyArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:55:6:55:24 | call to anyArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:55:6:55:24 | call to anyArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:57:6:57:34 | call to anyNamedArg | summaries.rb:1:20:1:36 | call to source : | summaries.rb:57:6:57:34 | call to anyNamedArg | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:61:6:61:39 | call to anyPositionFromOne | summaries.rb:1:20:1:36 | call to source : | summaries.rb:61:6:61:39 | call to anyPositionFromOne | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:64:8:64:8 | x | summaries.rb:1:20:1:36 | call to source : | summaries.rb:64:8:64:8 | x | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:71:8:71:54 | call to preserveTaint | summaries.rb:71:24:71:53 | call to source : | summaries.rb:71:8:71:54 | call to preserveTaint | $@ | summaries.rb:71:24:71:53 | call to source : | call to source : |
@@ -250,13 +264,15 @@ invalidSpecComponent
| summaries.rb:95:6:95:6 | y | summaries.rb:1:20:1:36 | call to source : | summaries.rb:95:6:95:6 | y | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:96:6:96:6 | z | summaries.rb:1:20:1:36 | call to source : | summaries.rb:96:6:96:6 | z | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:100:6:100:6 | x | summaries.rb:1:20:1:36 | call to source : | summaries.rb:100:6:100:6 | x | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:102:16:102:22 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:102:16:102:22 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:102:16:102:22 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:102:16:102:22 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:103:21:103:27 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:103:21:103:27 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
| summaries.rb:103:21:103:27 | tainted | summaries.rb:1:20:1:36 | call to source : | summaries.rb:103:21:103:27 | tainted | $@ | summaries.rb:1:20:1:36 | call to source : | call to source : |
warning
| CSV type row should have 5 columns but has 2: test;TooFewColumns |
| CSV type row should have 5 columns but has 8: test;TooManyColumns;;;Member[Foo].Instance;too;many;columns |
| Invalid argument '0-1' in token 'Argument[0-1]' in access path: Method[foo].Argument[0-1] |
| Invalid argument '*' in token 'Argument[*]' in access path: Method[foo].Argument[*] |
| Invalid argument 'any' in token 'Argument[any]' in access path: Member[Foo].Method[sinkAnyArg].Argument[any] |
| Invalid argument 'any-named' in token 'Argument[any-named]' in access path: Member[Foo].Method[sinkAnyNamedArg].Argument[any-named] |
| Invalid token 'Argument' is missing its arguments, in access path: Method[foo].Argument |
| Invalid token 'Member' is missing its arguments, in access path: Method[foo].Member |
| Invalid token name 'Arg' in access path: Method[foo].Arg[0] |

8
ruby/ql/test/library-tests/dataflow/summaries/summaries.rb
View File

@@ -54,7 +54,7 @@ sink(Foo.namedArg(tainted))
sink(Foo.anyArg(foo: tainted)) # $ hasTaintFlow=tainted
sink(Foo.anyArg(tainted)) # $ hasTaintFlow=tainted
sink(Foo.anyNamedArg(foo: tainted)) # $ MISSING: hasTaintFlow=tainted
sink(Foo.anyNamedArg(foo: tainted)) # $ hasTaintFlow=tainted
sink(Foo.anyNamedArg(tainted))
sink(Foo.anyPositionFromOne(tainted))
@@ -99,8 +99,8 @@ x = Foo.new
x.flowToSelf(tainted)
sink(x) # $ hasTaintFlow=tainted
Foo.sinkAnyArg(tainted) # $ MISSING: hasTaintFlow=tainted
Foo.sinkAnyArg(key: tainted) # $ MISSING: hasTaintFlow=tainted
Foo.sinkAnyArg(tainted) # $ hasValueFlow=tainted
Foo.sinkAnyArg(key: tainted) # $ hasValueFlow=tainted
Foo.sinkAnyNamedArg(tainted)
Foo.sinkAnyNamedArg(key: tainted) # $ MISSING: hasTaintFlow=tainted
Foo.sinkAnyNamedArg(key: tainted) # $ MISSING: hasValueFlow=tainted