Skip to content

Java, Ruby: add missing .qlref tests #19888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 27, 2025
Merged

Java, Ruby: add missing .qlref tests #19888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0b3a2c
Java: convert ArbitraryApkInstallation test to .qlref
d10c Jun 26, 2025
89f1ee0
Ruby: add meta/TaintedNodes.ql test
d10c Jun 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file removed java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.expected
View file
Open in desktop
Empty file.
19 changes: 0 additions & 19 deletions java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.ql
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions ...sts/security/CWE-094/ApkInstallation.java → .../ApkInstallationTest/ApkInstallation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,46 +11,46 @@ public class ApkInstallation extends Activity {
public void installAPK(String path) {
// BAD: the path is not checked
Intent intent = new Intent(Intent.ACTION_VIEW);
intent.setDataAndType(Uri.fromFile(new File(path)), "application/vnd.android.package-archive"); // $ hasApkInstallation
intent.setDataAndType(Uri.fromFile(new File(path)), "application/vnd.android.package-archive"); // $ Alert
startActivity(intent);
}

public void installAPK3(String path) {
Intent intent = new Intent(Intent.ACTION_VIEW);
intent.setType(APK_MIMETYPE);
// BAD: the path is not checked
intent.setData(Uri.fromFile(new File(path))); // $ hasApkInstallation
intent.setData(Uri.fromFile(new File(path))); // $ Alert
startActivity(intent);
}

public void installAPKFromExternalStorage(String path) {
// BAD: file is from external storage
File file = new File(Environment.getExternalStorageDirectory(), path);
Intent intent = new Intent(Intent.ACTION_VIEW);
intent.setDataAndType(Uri.fromFile(file), APK_MIMETYPE); // $ hasApkInstallation
intent.setDataAndType(Uri.fromFile(file), APK_MIMETYPE); // $ Alert
startActivity(intent);
}

public void installAPKFromExternalStorageWithActionInstallPackage(String path) {
// BAD: file is from external storage
File file = new File(Environment.getExternalStorageDirectory(), path);
Intent intent = new Intent(Intent.ACTION_INSTALL_PACKAGE);
intent.setData(Uri.fromFile(file)); // $ hasApkInstallation
intent.setData(Uri.fromFile(file)); // $ Alert
startActivity(intent);
}

public void installAPKInstallPackageLiteral(String path) {
File file = new File(Environment.getExternalStorageDirectory(), path);
Intent intent = new Intent("android.intent.action.INSTALL_PACKAGE");
intent.setData(Uri.fromFile(file)); // $ hasApkInstallation
intent.setData(Uri.fromFile(file)); // $ Alert
startActivity(intent);
}

public void otherIntent(File file) {
Intent intent = new Intent(this, OtherActivity.class);
intent.setAction(Intent.ACTION_VIEW);
// BAD: the file is from unknown source
intent.setData(Uri.fromFile(file)); // $ hasApkInstallation
intent.setData(Uri.fromFile(file)); // $ Alert
}
}

Expand Down
16 changes: 16 additions & 0 deletions java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#select
| 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. |
| 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. |
| 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. |
| 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. |
| 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. |
| 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. |
edges
nodes
| ApkInstallation.java:14:31:14:58 | fromFile(...) | semmle.label | fromFile(...) |
| ApkInstallation.java:22:24:22:51 | fromFile(...) | semmle.label | fromFile(...) |
| ApkInstallation.java:30:31:30:48 | fromFile(...) | semmle.label | fromFile(...) |
| ApkInstallation.java:38:24:38:41 | fromFile(...) | semmle.label | fromFile(...) |
| ApkInstallation.java:45:24:45:41 | fromFile(...) | semmle.label | fromFile(...) |
| ApkInstallation.java:53:24:53:41 | fromFile(...) | semmle.label | fromFile(...) |
subpaths
4 changes: 4 additions & 0 deletions java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.qlref
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
query: Security/CWE/CWE-094/ArbitraryApkInstallation.ql
postprocess:
- utils/test/PrettyPrintModels.ql
- utils/test/InlineExpectationsTestQuery.ql
1 change: 1 addition & 0 deletions java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/options
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +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
79 changes: 79 additions & 0 deletions ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
| tainted_path.rb:4:5:4:24 | ... = ... | Tainted node |
| tainted_path.rb:4:12:4:17 | call to params | Tainted node |
| tainted_path.rb:4:12:4:24 | ...[...] | Tainted node |
| tainted_path.rb:5:26:5:29 | path | Tainted node |
| tainted_path.rb:10:5:10:43 | ... = ... | Tainted node |
| tainted_path.rb:10:12:10:43 | call to absolute_path | Tainted node |
| tainted_path.rb:10:31:10:36 | call to params | Tainted node |
| tainted_path.rb:10:31:10:43 | ...[...] | Tainted node |
| tainted_path.rb:11:26:11:29 | path | Tainted node |
| tainted_path.rb:16:5:16:47 | ... = ... | Tainted node |
| tainted_path.rb:16:12:16:47 | "#{...}/foo" | Tainted node |
| tainted_path.rb:16:13:16:42 | #{...} | Tainted node |
| tainted_path.rb:16:15:16:41 | call to dirname | Tainted node |
| tainted_path.rb:16:28:16:33 | call to params | Tainted node |
| tainted_path.rb:16:28:16:40 | ...[...] | Tainted node |
| tainted_path.rb:17:26:17:29 | path | Tainted node |
| tainted_path.rb:22:5:22:41 | ... = ... | Tainted node |
| tainted_path.rb:22:12:22:41 | call to expand_path | Tainted node |
| tainted_path.rb:22:29:22:34 | call to params | Tainted node |
| tainted_path.rb:22:29:22:41 | ...[...] | Tainted node |
| tainted_path.rb:23:26:23:29 | path | Tainted node |
| tainted_path.rb:28:5:28:34 | ... = ... | Tainted node |
| tainted_path.rb:28:12:28:34 | call to path | Tainted node |
| tainted_path.rb:28:22:28:27 | call to params | Tainted node |
| tainted_path.rb:28:22:28:34 | ...[...] | Tainted node |
| tainted_path.rb:29:26:29:29 | path | Tainted node |
| tainted_path.rb:34:5:34:41 | ... = ... | Tainted node |
| tainted_path.rb:34:12:34:41 | call to realdirpath | Tainted node |
| tainted_path.rb:34:29:34:34 | call to params | Tainted node |
| tainted_path.rb:34:29:34:41 | ...[...] | Tainted node |
| tainted_path.rb:35:26:35:29 | path | Tainted node |
| tainted_path.rb:40:5:40:38 | ... = ... | Tainted node |
| tainted_path.rb:40:12:40:38 | call to realpath | Tainted node |
| tainted_path.rb:40:26:40:31 | call to params | Tainted node |
| tainted_path.rb:40:26:40:38 | ...[...] | Tainted node |
| tainted_path.rb:41:26:41:29 | path | Tainted node |
| tainted_path.rb:47:5:47:63 | ... = ... | Tainted node |
| tainted_path.rb:47:12:47:63 | call to join | Tainted node |
| tainted_path.rb:47:43:47:48 | call to params | Tainted node |
| tainted_path.rb:47:43:47:55 | ...[...] | Tainted node |
| tainted_path.rb:48:26:48:29 | path | Tainted node |
| tainted_path.rb:53:26:53:31 | call to params | Tainted node |
| tainted_path.rb:53:26:53:38 | ...[...] | Tainted node |
| tainted_path.rb:59:5:59:53 | ... = ... | Tainted node |
| tainted_path.rb:59:12:59:53 | call to new | Tainted node |
| tainted_path.rb:59:40:59:45 | call to params | Tainted node |
| tainted_path.rb:59:40:59:52 | ...[...] | Tainted node |
| tainted_path.rb:60:26:60:29 | path | Tainted node |
| tainted_path.rb:65:5:65:63 | ... = ... | Tainted node |
| tainted_path.rb:65:12:65:53 | call to new | Tainted node |
| tainted_path.rb:65:12:65:63 | call to sanitized | Tainted node |
| tainted_path.rb:65:40:65:45 | call to params | Tainted node |
| tainted_path.rb:65:40:65:52 | ...[...] | Tainted node |
| tainted_path.rb:66:26:66:29 | path | Tainted node |
| tainted_path.rb:71:5:71:53 | ... = ... | Tainted node |
| tainted_path.rb:71:12:71:53 | call to new | Tainted node |
| tainted_path.rb:71:40:71:45 | call to params | Tainted node |
| tainted_path.rb:71:40:71:52 | ...[...] | Tainted node |
| tainted_path.rb:72:15:72:18 | path | Tainted node |
| tainted_path.rb:77:5:77:53 | ... = ... | Tainted node |
| tainted_path.rb:77:12:77:53 | call to new | Tainted node |
| tainted_path.rb:77:40:77:45 | call to params | Tainted node |
| tainted_path.rb:77:40:77:52 | ...[...] | Tainted node |
| tainted_path.rb:78:19:78:22 | path | Tainted node |
| tainted_path.rb:79:14:79:17 | path | Tainted node |
| tainted_path.rb:84:5:84:53 | ... = ... | Tainted node |
| tainted_path.rb:84:12:84:53 | call to new | Tainted node |
| tainted_path.rb:84:40:84:45 | call to params | Tainted node |
| tainted_path.rb:84:40:84:52 | ...[...] | Tainted node |
| tainted_path.rb:85:10:85:13 | path | Tainted node |
| tainted_path.rb:86:25:86:28 | path | Tainted node |
| tainted_path.rb:90:5:90:53 | ... = ... | Tainted node |
| tainted_path.rb:90:12:90:53 | call to new | Tainted node |
| tainted_path.rb:90:40:90:45 | call to params | Tainted node |
| tainted_path.rb:90:40:90:52 | ...[...] | Tainted node |
| tainted_path.rb:91:10:91:43 | "Debug: require_relative(#{...})" | Tainted node |
| tainted_path.rb:91:35:91:41 | #{...} | Tainted node |
| tainted_path.rb:91:37:91:40 | path | Tainted node |
| tainted_path.rb:92:11:92:14 | path | Tainted node |
4 changes: 4 additions & 0 deletions ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.qlref
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
query: queries/meta/TaintedNodes.ql
postprocess:
- utils/test/PrettyPrintModels.ql
- utils/test/InlineExpectationsTestQuery.ql
94 changes: 94 additions & 0 deletions ruby/ql/test/query-tests/meta/TaintedNodes/tainted_path.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
class FooController < ActionController::Base
# BAD
def route0
path = params[:path] # $ Alert
@content = File.read path # $ Alert
end

# BAD - File.absolute_path preserves taint
def route1
path = File.absolute_path params[:path] # $ Alert
@content = File.read path # $ Alert
end

# BAD - File.dirname preserves taint
def route2
path = "#{File.dirname(params[:path])}/foo" # $ Alert
@content = File.read path # $ Alert
end

# BAD - File.expand_path preserves taint
def route3
path = File.expand_path params[:path] # $ Alert
@content = File.read path # $ Alert
end

# BAD - File.path preserves taint
def route4
path = File.path params[:path] # $ Alert
@content = File.read path # $ Alert
end

# BAD - File.realdirpath preserves taint
def route5
path = File.realdirpath params[:path] # $ Alert
@content = File.read path # $ Alert
end

# BAD - File.realpath preserves taint
def route6
path = File.realpath params[:path] # $ Alert
@content = File.read path # $ Alert
end

# BAD - tainted arguments in any position propagate to the return value of
# File.join
def route7
path = File.join("foo", "bar", "baz", params[:path], "qux") # $ Alert
@content = File.read path # $ Alert
end

# GOOD - File.basename does not preserve taint
def route8
path = File.basename params[:path] # $ Alert
@content = File.read path # Sanitized
end

# BAD
def route9
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
@content = File.read path # $ Alert
end

# GOOD - explicitly sanitized
def route10
path = ActiveStorage::Filename.new(params[:path]).sanitized # $ Alert
@content = File.read path # $ SPURIOUS: Alert (should have been sanitized)
end

# BAD
def route11
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
send_file path # $ Alert
end

# BAD
def route12
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
bla (Dir.glob path) # $ Alert
bla (Dir[path]) # $ Alert
end

# BAD
def route13
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
load(path) # $ Alert
autoload(:MyModule, path) # $ Alert
end

def require_relative()
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
Comment on lines +89 to +90
Copy link
Preview

Copilot AI Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The require_relative method is defined without a path parameter but calls super(path). Update the signature to def require_relative(path) to match the super call.

Suggested change
def require_relative()
path = ActiveStorage::Filename.new(params[:path]) # $ Alert
def require_relative(path)

Copilot uses AI. Check for mistakes.

puts "Debug: require_relative(#{path})" # $ Alert
super(path) # $ Alert
end
end
Loading