diff --git a/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.expected b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.expected deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.ql b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.ql deleted file mode 100644 index a4efceebc189..000000000000 --- a/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest.ql +++ /dev/null @@ -1,19 +0,0 @@ -import java -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.security.ArbitraryApkInstallationQuery -import utils.test.InlineExpectationsTest - -module HasApkInstallationTest implements TestSig { - string getARelevantTag() { result = "hasApkInstallation" } - - predicate hasActualResult(Location location, string element, string tag, string value) { - tag = "hasApkInstallation" and - exists(DataFlow::Node sink | ApkInstallationFlow::flowTo(sink) | - sink.getLocation() = location and - element = sink.toString() and - value = "" - ) - } -} - -import MakeTest diff --git a/java/ql/test/query-tests/security/CWE-094/ApkInstallation.java b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallation.java similarity index 83% rename from java/ql/test/query-tests/security/CWE-094/ApkInstallation.java rename to java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallation.java index 680ad6330839..ee6a0c56b709 100644 --- a/java/ql/test/query-tests/security/CWE-094/ApkInstallation.java +++ b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallation.java @@ -11,7 +11,7 @@ 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); } @@ -19,7 +19,7 @@ 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); } @@ -27,7 +27,7 @@ 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); } @@ -35,14 +35,14 @@ 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); } @@ -50,7 +50,7 @@ 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 } } diff --git a/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.expected b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.expected new file mode 100644 index 000000000000..7a6b0ccde886 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.expected @@ -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 diff --git a/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.qlref b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.qlref new file mode 100644 index 000000000000..7566db8af78d --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/ApkInstallationTest.qlref @@ -0,0 +1,4 @@ +query: Security/CWE/CWE-094/ArbitraryApkInstallation.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/options b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/options new file mode 100644 index 000000000000..d7c8332682ba --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-094/ApkInstallationTest/options @@ -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 diff --git a/ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.expected b/ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.expected new file mode 100644 index 000000000000..26956a4ad927 --- /dev/null +++ b/ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.expected @@ -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 | diff --git a/ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.qlref b/ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.qlref new file mode 100644 index 000000000000..0fd4f30b68ae --- /dev/null +++ b/ruby/ql/test/query-tests/meta/TaintedNodes/TaintedNodes.qlref @@ -0,0 +1,4 @@ +query: queries/meta/TaintedNodes.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/ruby/ql/test/query-tests/meta/TaintedNodes/tainted_path.rb b/ruby/ql/test/query-tests/meta/TaintedNodes/tainted_path.rb new file mode 100644 index 000000000000..a85c0ad975d2 --- /dev/null +++ b/ruby/ql/test/query-tests/meta/TaintedNodes/tainted_path.rb @@ -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 + puts "Debug: require_relative(#{path})" # $ Alert + super(path) # $ Alert + end +end