Skip to content

Commit 66946eb

Browse files
committed
add Kernel methods as sinks to path-injection
1 parent be16890 commit 66946eb

File tree

3 files changed

+33
-0
lines changed

3 files changed

+33
-0
lines changed

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,19 @@ module Kernel {
179179
preservesValue = true
180180
}
181181
}
182+
183+
/** A call to e.g. `Kernel.load` that accesses a file. */
184+
private class KernelFileAccess extends FileSystemAccess::Range instanceof KernelMethodCall {
185+
KernelFileAccess() {
186+
super.getMethodName() = ["load", "require", "require_relative", "autoload", "autoload?"]
187+
}
188+
189+
override DataFlow::Node getAPathArgument() {
190+
result = super.getArgument(0) and
191+
super.getMethodName() = ["load", "require", "require_relative"]
192+
or
193+
result = super.getArgument(1) and
194+
super.getMethodName() = ["autoload", "autoload?"]
195+
}
196+
}
182197
}

ruby/ql/test/query-tests/security/cwe-022/PathInjection.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ edges
4343
| tainted_path.rb:77:12:77:53 | call to new : | tainted_path.rb:79:14:79:17 | path |
4444
| tainted_path.rb:77:40:77:45 | call to params : | tainted_path.rb:77:40:77:52 | ...[...] : |
4545
| tainted_path.rb:77:40:77:52 | ...[...] : | tainted_path.rb:77:12:77:53 | call to new : |
46+
| tainted_path.rb:84:12:84:53 | call to new : | tainted_path.rb:85:10:85:13 | path |
47+
| tainted_path.rb:84:12:84:53 | call to new : | tainted_path.rb:86:25:86:28 | path |
48+
| tainted_path.rb:84:40:84:45 | call to params : | tainted_path.rb:84:40:84:52 | ...[...] : |
49+
| tainted_path.rb:84:40:84:52 | ...[...] : | tainted_path.rb:84:12:84:53 | call to new : |
4650
nodes
4751
| ArchiveApiPathTraversal.rb:5:26:5:31 | call to params : | semmle.label | call to params : |
4852
| ArchiveApiPathTraversal.rb:5:26:5:42 | ...[...] : | semmle.label | ...[...] : |
@@ -102,6 +106,11 @@ nodes
102106
| tainted_path.rb:77:40:77:52 | ...[...] : | semmle.label | ...[...] : |
103107
| tainted_path.rb:78:19:78:22 | path | semmle.label | path |
104108
| tainted_path.rb:79:14:79:17 | path | semmle.label | path |
109+
| tainted_path.rb:84:12:84:53 | call to new : | semmle.label | call to new : |
110+
| tainted_path.rb:84:40:84:45 | call to params : | semmle.label | call to params : |
111+
| tainted_path.rb:84:40:84:52 | ...[...] : | semmle.label | ...[...] : |
112+
| tainted_path.rb:85:10:85:13 | path | semmle.label | path |
113+
| tainted_path.rb:86:25:86:28 | path | semmle.label | path |
105114
subpaths
106115
#select
107116
| ArchiveApiPathTraversal.rb:59:21:59:36 | destination_file | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params : | ArchiveApiPathTraversal.rb:59:21:59:36 | destination_file | This path depends on a $@. | ArchiveApiPathTraversal.rb:5:26:5:31 | call to params | user-provided value |
@@ -119,3 +128,5 @@ subpaths
119128
| tainted_path.rb:72:15:72:18 | path | tainted_path.rb:71:40:71:45 | call to params : | tainted_path.rb:72:15:72:18 | path | This path depends on a $@. | tainted_path.rb:71:40:71:45 | call to params | user-provided value |
120129
| tainted_path.rb:78:19:78:22 | path | tainted_path.rb:77:40:77:45 | call to params : | tainted_path.rb:78:19:78:22 | path | This path depends on a $@. | tainted_path.rb:77:40:77:45 | call to params | user-provided value |
121130
| tainted_path.rb:79:14:79:17 | path | tainted_path.rb:77:40:77:45 | call to params : | tainted_path.rb:79:14:79:17 | path | This path depends on a $@. | tainted_path.rb:77:40:77:45 | call to params | user-provided value |
131+
| tainted_path.rb:85:10:85:13 | path | tainted_path.rb:84:40:84:45 | call to params : | tainted_path.rb:85:10:85:13 | path | This path depends on a $@. | tainted_path.rb:84:40:84:45 | call to params | user-provided value |
132+
| tainted_path.rb:86:25:86:28 | path | tainted_path.rb:84:40:84:45 | call to params : | tainted_path.rb:86:25:86:28 | path | This path depends on a $@. | tainted_path.rb:84:40:84:45 | call to params | user-provided value |

ruby/ql/test/query-tests/security/cwe-022/tainted_path.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,11 @@ def route12
7878
bla (Dir.glob path)
7979
bla (Dir[path])
8080
end
81+
82+
# BAD
83+
def route13
84+
path = ActiveStorage::Filename.new(params[:path])
85+
load(path)
86+
autoload(:MyModule, path)
87+
end
8188
end

0 commit comments

Comments
 (0)