Skip to content

Commit f568d41

Browse files
d10cd10c
authored
Merge pull request #19888 from d10c/d10c/missing-diff-informed-tests
Java, Ruby: add missing .qlref tests
2 parents db0fc7b + 89f1ee0 commit f568d41

File tree

9 files changed

+204
-25
lines changed

9 files changed

+204
-25
lines changed

‎java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.expected

Whitespace-only changes.

‎java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.ql

Lines changed: 0 additions & 19 deletions
This file was deleted.

‎java/ql/test/query-tests/security/CWE-094/ApkInstallation.java renamed to ‎java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallation.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,46 +11,46 @@ public class ApkInstallation extends Activity {
1111
public void installAPK(String path) {
1212
// BAD: the path is not checked
1313
Intent intent = new Intent(Intent.ACTION_VIEW);
14-
intent.setDataAndType(Uri.fromFile(new File(path)), "application/vnd.android.package-archive"); // $ hasApkInstallation
14+
intent.setDataAndType(Uri.fromFile(new File(path)), "application/vnd.android.package-archive"); // $ Alert
1515
startActivity(intent);
1616
}
1717

1818
public void installAPK3(String path) {
1919
Intent intent = new Intent(Intent.ACTION_VIEW);
2020
intent.setType(APK_MIMETYPE);
2121
// BAD: the path is not checked
22-
intent.setData(Uri.fromFile(new File(path))); // $ hasApkInstallation
22+
intent.setData(Uri.fromFile(new File(path))); // $ Alert
2323
startActivity(intent);
2424
}
2525

2626
public void installAPKFromExternalStorage(String path) {
2727
// BAD: file is from external storage
2828
File file = new File(Environment.getExternalStorageDirectory(), path);
2929
Intent intent = new Intent(Intent.ACTION_VIEW);
30-
intent.setDataAndType(Uri.fromFile(file), APK_MIMETYPE); // $ hasApkInstallation
30+
intent.setDataAndType(Uri.fromFile(file), APK_MIMETYPE); // $ Alert
3131
startActivity(intent);
3232
}
3333

3434
public void installAPKFromExternalStorageWithActionInstallPackage(String path) {
3535
// BAD: file is from external storage
3636
File file = new File(Environment.getExternalStorageDirectory(), path);
3737
Intent intent = new Intent(Intent.ACTION_INSTALL_PACKAGE);
38-
intent.setData(Uri.fromFile(file)); // $ hasApkInstallation
38+
intent.setData(Uri.fromFile(file)); // $ Alert
3939
startActivity(intent);
4040
}
4141

4242
public void installAPKInstallPackageLiteral(String path) {
4343
File file = new File(Environment.getExternalStorageDirectory(), path);
4444
Intent intent = new Intent("android.intent.action.INSTALL_PACKAGE");
45-
intent.setData(Uri.fromFile(file)); // $ hasApkInstallation
45+
intent.setData(Uri.fromFile(file)); // $ Alert
4646
startActivity(intent);
4747
}
4848

4949
public void otherIntent(File file) {
5050
Intent intent = new Intent(this, OtherActivity.class);
5151
intent.setAction(Intent.ACTION_VIEW);
5252
// BAD: the file is from unknown source
53-
intent.setData(Uri.fromFile(file)); // $ hasApkInstallation
53+
intent.setData(Uri.fromFile(file)); // $ Alert
5454
}
5555
}
5656

‎java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#select
2+
| ApkInstallation.java:14:31:14:58 | fromFile(...) | ApkInstallation.java:14:31:14:58 | fromFile(...) | ApkInstallation.java:14:31:14:58 | fromFile(...) | Arbitrary Android APK installation. |
3+
| ApkInstallation.java:22:24:22:51 | fromFile(...) | ApkInstallation.java:22:24:22:51 | fromFile(...) | ApkInstallation.java:22:24:22:51 | fromFile(...) | Arbitrary Android APK installation. |
4+
| ApkInstallation.java:30:31:30:48 | fromFile(...) | ApkInstallation.java:30:31:30:48 | fromFile(...) | ApkInstallation.java:30:31:30:48 | fromFile(...) | Arbitrary Android APK installation. |
5+
| ApkInstallation.java:38:24:38:41 | fromFile(...) | ApkInstallation.java:38:24:38:41 | fromFile(...) | ApkInstallation.java:38:24:38:41 | fromFile(...) | Arbitrary Android APK installation. |
6+
| ApkInstallation.java:45:24:45:41 | fromFile(...) | ApkInstallation.java:45:24:45:41 | fromFile(...) | ApkInstallation.java:45:24:45:41 | fromFile(...) | Arbitrary Android APK installation. |
7+
| ApkInstallation.java:53:24:53:41 | fromFile(...) | ApkInstallation.java:53:24:53:41 | fromFile(...) | ApkInstallation.java:53:24:53:41 | fromFile(...) | Arbitrary Android APK installation. |
8+
edges
9+
nodes
10+
| ApkInstallation.java:14:31:14:58 | fromFile(...) | semmle.label | fromFile(...) |
11+
| ApkInstallation.java:22:24:22:51 | fromFile(...) | semmle.label | fromFile(...) |
12+
| ApkInstallation.java:30:31:30:48 | fromFile(...) | semmle.label | fromFile(...) |
13+
| ApkInstallation.java:38:24:38:41 | fromFile(...) | semmle.label | fromFile(...) |
14+
| ApkInstallation.java:45:24:45:41 | fromFile(...) | semmle.label | fromFile(...) |
15+
| ApkInstallation.java:53:24:53:41 | fromFile(...) | semmle.label | fromFile(...) |
16+
subpaths

‎java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.qlref

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
query: Security/CWE/CWE-094/ArbitraryApkInstallation.ql
2+
postprocess:
3+
- utils/test/PrettyPrintModels.ql
4+
- utils/test/InlineExpectationsTestQuery.ql

‎java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/options

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/validation-api-2.0.1.Final:${testdir}/../../../../stubs/springframework-5.8.x:${testdir}/../../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../../stubs/apache-commons-jexl-3.1:${testdir}/../../../../stubs/apache-commons-logging-1.2:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/apache-freemarker-2.3.31:${testdir}/../../../../stubs/jinjava-2.6.0:${testdir}/../../../../stubs/pebble-3.1.5:${testdir}/../../../../stubs/thymeleaf-3.0.14:${testdir}/../../../../stubs/apache-velocity-2.3:${testdir}/../../../..//stubs/google-android-9.0.0

‎ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.expected

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
| tainted_path.rb:4:5:4:24 | ... = ... | Tainted node |
2+
| tainted_path.rb:4:12:4:17 | call to params | Tainted node |
3+
| tainted_path.rb:4:12:4:24 | ...[...] | Tainted node |
4+
| tainted_path.rb:5:26:5:29 | path | Tainted node |
5+
| tainted_path.rb:10:5:10:43 | ... = ... | Tainted node |
6+
| tainted_path.rb:10:12:10:43 | call to absolute_path | Tainted node |
7+
| tainted_path.rb:10:31:10:36 | call to params | Tainted node |
8+
| tainted_path.rb:10:31:10:43 | ...[...] | Tainted node |
9+
| tainted_path.rb:11:26:11:29 | path | Tainted node |
10+
| tainted_path.rb:16:5:16:47 | ... = ... | Tainted node |
11+
| tainted_path.rb:16:12:16:47 | "#{...}/foo" | Tainted node |
12+
| tainted_path.rb:16:13:16:42 | #{...} | Tainted node |
13+
| tainted_path.rb:16:15:16:41 | call to dirname | Tainted node |
14+
| tainted_path.rb:16:28:16:33 | call to params | Tainted node |
15+
| tainted_path.rb:16:28:16:40 | ...[...] | Tainted node |
16+
| tainted_path.rb:17:26:17:29 | path | Tainted node |
17+
| tainted_path.rb:22:5:22:41 | ... = ... | Tainted node |
18+
| tainted_path.rb:22:12:22:41 | call to expand_path | Tainted node |
19+
| tainted_path.rb:22:29:22:34 | call to params | Tainted node |
20+
| tainted_path.rb:22:29:22:41 | ...[...] | Tainted node |
21+
| tainted_path.rb:23:26:23:29 | path | Tainted node |
22+
| tainted_path.rb:28:5:28:34 | ... = ... | Tainted node |
23+
| tainted_path.rb:28:12:28:34 | call to path | Tainted node |
24+
| tainted_path.rb:28:22:28:27 | call to params | Tainted node |
25+
| tainted_path.rb:28:22:28:34 | ...[...] | Tainted node |
26+
| tainted_path.rb:29:26:29:29 | path | Tainted node |
27+
| tainted_path.rb:34:5:34:41 | ... = ... | Tainted node |
28+
| tainted_path.rb:34:12:34:41 | call to realdirpath | Tainted node |
29+
| tainted_path.rb:34:29:34:34 | call to params | Tainted node |
30+
| tainted_path.rb:34:29:34:41 | ...[...] | Tainted node |
31+
| tainted_path.rb:35:26:35:29 | path | Tainted node |
32+
| tainted_path.rb:40:5:40:38 | ... = ... | Tainted node |
33+
| tainted_path.rb:40:12:40:38 | call to realpath | Tainted node |
34+
| tainted_path.rb:40:26:40:31 | call to params | Tainted node |
35+
| tainted_path.rb:40:26:40:38 | ...[...] | Tainted node |
36+
| tainted_path.rb:41:26:41:29 | path | Tainted node |
37+
| tainted_path.rb:47:5:47:63 | ... = ... | Tainted node |
38+
| tainted_path.rb:47:12:47:63 | call to join | Tainted node |
39+
| tainted_path.rb:47:43:47:48 | call to params | Tainted node |
40+
| tainted_path.rb:47:43:47:55 | ...[...] | Tainted node |
41+
| tainted_path.rb:48:26:48:29 | path | Tainted node |
42+
| tainted_path.rb:53:26:53:31 | call to params | Tainted node |
43+
| tainted_path.rb:53:26:53:38 | ...[...] | Tainted node |
44+
| tainted_path.rb:59:5:59:53 | ... = ... | Tainted node |
45+
| tainted_path.rb:59:12:59:53 | call to new | Tainted node |
46+
| tainted_path.rb:59:40:59:45 | call to params | Tainted node |
47+
| tainted_path.rb:59:40:59:52 | ...[...] | Tainted node |
48+
| tainted_path.rb:60:26:60:29 | path | Tainted node |
49+
| tainted_path.rb:65:5:65:63 | ... = ... | Tainted node |
50+
| tainted_path.rb:65:12:65:53 | call to new | Tainted node |
51+
| tainted_path.rb:65:12:65:63 | call to sanitized | Tainted node |
52+
| tainted_path.rb:65:40:65:45 | call to params | Tainted node |
53+
| tainted_path.rb:65:40:65:52 | ...[...] | Tainted node |
54+
| tainted_path.rb:66:26:66:29 | path | Tainted node |
55+
| tainted_path.rb:71:5:71:53 | ... = ... | Tainted node |
56+
| tainted_path.rb:71:12:71:53 | call to new | Tainted node |
57+
| tainted_path.rb:71:40:71:45 | call to params | Tainted node |
58+
| tainted_path.rb:71:40:71:52 | ...[...] | Tainted node |
59+
| tainted_path.rb:72:15:72:18 | path | Tainted node |
60+
| tainted_path.rb:77:5:77:53 | ... = ... | Tainted node |
61+
| tainted_path.rb:77:12:77:53 | call to new | Tainted node |
62+
| tainted_path.rb:77:40:77:45 | call to params | Tainted node |
63+
| tainted_path.rb:77:40:77:52 | ...[...] | Tainted node |
64+
| tainted_path.rb:78:19:78:22 | path | Tainted node |
65+
| tainted_path.rb:79:14:79:17 | path | Tainted node |
66+
| tainted_path.rb:84:5:84:53 | ... = ... | Tainted node |
67+
| tainted_path.rb:84:12:84:53 | call to new | Tainted node |
68+
| tainted_path.rb:84:40:84:45 | call to params | Tainted node |
69+
| tainted_path.rb:84:40:84:52 | ...[...] | Tainted node |
70+
| tainted_path.rb:85:10:85:13 | path | Tainted node |
71+
| tainted_path.rb:86:25:86:28 | path | Tainted node |
72+
| tainted_path.rb:90:5:90:53 | ... = ... | Tainted node |
73+
| tainted_path.rb:90:12:90:53 | call to new | Tainted node |
74+
| tainted_path.rb:90:40:90:45 | call to params | Tainted node |
75+
| tainted_path.rb:90:40:90:52 | ...[...] | Tainted node |
76+
| tainted_path.rb:91:10:91:43 | "Debug: require_relative(#{...})" | Tainted node |
77+
| tainted_path.rb:91:35:91:41 | #{...} | Tainted node |
78+
| tainted_path.rb:91:37:91:40 | path | Tainted node |
79+
| tainted_path.rb:92:11:92:14 | path | Tainted node |

‎ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.qlref

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
query: queries/meta/TaintedNodes.ql
2+
postprocess:
3+
- utils/test/PrettyPrintModels.ql
4+
- utils/test/InlineExpectationsTestQuery.ql

‎ruby/ql/test/query-tests/meta/TaintedNodes/tainted_path.rb

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
class FooController < ActionController::Base
2+
# BAD
3+
def route0
4+
path = params[:path] # $ Alert
5+
@content = File.read path # $ Alert
6+
end
7+
8+
# BAD - File.absolute_path preserves taint
9+
def route1
10+
path = File.absolute_path params[:path] # $ Alert
11+
@content = File.read path # $ Alert
12+
end
13+
14+
# BAD - File.dirname preserves taint
15+
def route2
16+
path = "#{File.dirname(params[:path])}/foo" # $ Alert
17+
@content = File.read path # $ Alert
18+
end
19+
20+
# BAD - File.expand_path preserves taint
21+
def route3
22+
path = File.expand_path params[:path] # $ Alert
23+
@content = File.read path # $ Alert
24+
end
25+
26+
# BAD - File.path preserves taint
27+
def route4
28+
path = File.path params[:path] # $ Alert
29+
@content = File.read path # $ Alert
30+
end
31+
32+
# BAD - File.realdirpath preserves taint
33+
def route5
34+
path = File.realdirpath params[:path] # $ Alert
35+
@content = File.read path # $ Alert
36+
end
37+
38+
# BAD - File.realpath preserves taint
39+
def route6
40+
path = File.realpath params[:path] # $ Alert
41+
@content = File.read path # $ Alert
42+
end
43+
44+
# BAD - tainted arguments in any position propagate to the return value of
45+
# File.join
46+
def route7
47+
path = File.join("foo", "bar", "baz", params[:path], "qux") # $ Alert
48+
@content = File.read path # $ Alert
49+
end
50+
51+
# GOOD - File.basename does not preserve taint
52+
def route8
53+
path = File.basename params[:path] # $ Alert
54+
@content = File.read path # Sanitized
55+
end
56+
57+
# BAD
58+
def route9
59+
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
60+
@content = File.read path # $ Alert
61+
end
62+
63+
# GOOD - explicitly sanitized
64+
def route10
65+
path = ActiveStorage::Filename.new(params[:path]).sanitized # $ Alert
66+
@content = File.read path # $ SPURIOUS: Alert (should have been sanitized)
67+
end
68+
69+
# BAD
70+
def route11
71+
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
72+
send_file path # $ Alert
73+
end
74+
75+
# BAD
76+
def route12
77+
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
78+
bla (Dir.glob path) # $ Alert
79+
bla (Dir[path]) # $ Alert
80+
end
81+
82+
# BAD
83+
def route13
84+
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
85+
load(path) # $ Alert
86+
autoload(:MyModule, path) # $ Alert
87+
end
88+
89+
def require_relative()
90+
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
91+
puts "Debug: require_relative(#{path})" # $ Alert
92+
super(path) # $ Alert
93+
end
94+
end

0 commit comments

Comments
 (0)