Merge pull request #18645 from fabienpe/main

Added missing "GOOD" and "BAD" to some examples

Author: owen-mc

java/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.java

@@ -4,6 +4,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
 
 		StringBuilder sqlQueryBuilder = new StringBuilder();
 		sqlQueryBuilder.append("SELECT * FROM user WHERE user_id='");
+		// BAD: a request parameter is concatenated directly into a SQL query
 		sqlQueryBuilder.append(request.getParameter("user_id"));
 		sqlQueryBuilder.append("'");
 

java/ql/src/Security/CWE/CWE-023/PartialPathTraversalBad.java

@@ -1,5 +1,6 @@
 public class PartialPathTraversalBad {
     public void example(File dir, File parent) throws IOException {
+        // BAD: dir.getCanonicalPath() not slash-terminated
         if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath())) {
             throw new IOException("Path traversal attempt: " + dir.getCanonicalPath());
         }

java/ql/src/Security/CWE/CWE-023/PartialPathTraversalGood.java

@@ -2,6 +2,7 @@
 
 public class PartialPathTraversalGood {
     public void example(File dir, File parent) throws IOException {
+        // GOOD: Check if dir.Path() is normalised
         if (!dir.toPath().normalize().startsWith(parent.toPath())) {
             throw new IOException("Path traversal attempt: " + dir.getCanonicalPath());
         }

java/ql/src/Security/CWE/CWE-079/AndroidWebViewAddJavascriptInterfaceExample.java

@@ -20,4 +20,5 @@ public String studentEmail(String studentName) {
 webview.loadData("", "text/html", null);
 
 String name = "Robert'; DROP TABLE students; --";
+// BAD: Untrusted input loaded into WebView
 webview.loadUrl("javascript:alert(exposedObject.studentEmail(\""+ name +"\"))");

java/ql/src/Security/CWE/CWE-079/WebSettingsDisableJavascript.java

@@ -1,2 +1,2 @@
 WebSettings settings = webview.getSettings();
-settings.setJavaScriptEnabled(false);
+settings.setJavaScriptEnabled(false); // GOOD: webview has JavaScript disabled

java/ql/src/Security/CWE/CWE-079/WebSettingsEnableJavascript.java

@@ -1,2 +1,2 @@
 WebSettings settings = webview.getSettings();
-settings.setJavaScriptEnabled(true);
+settings.setJavaScriptEnabled(true); // BAD: webview has JavaScript enabled

java/ql/src/Security/CWE/CWE-094/GroovyInjectionBad.java

@@ -2,26 +2,26 @@ public class GroovyInjection {
     void injectionViaClassLoader(HttpServletRequest request) {    
         String script = request.getParameter("script");
         final GroovyClassLoader classLoader = new GroovyClassLoader();
-        Class groovy = classLoader.parseClass(script);
+        Class groovy = classLoader.parseClass(script); // BAD: Groovy code injection
         GroovyObject groovyObj = (GroovyObject) groovy.newInstance();
     }
 
     void injectionViaEval(HttpServletRequest request) {
         String script = request.getParameter("script");
-        Eval.me(script);
+        Eval.me(script); // BAD: Groovy code injection
     }
 
     void injectionViaGroovyShell(HttpServletRequest request) {
         GroovyShell shell = new GroovyShell();
         String script = request.getParameter("script");
-        shell.evaluate(script);
+        shell.evaluate(script); // BAD: Groovy code injection
     }
 
     void injectionViaGroovyShellGroovyCodeSource(HttpServletRequest request) {
         GroovyShell shell = new GroovyShell();
         String script = request.getParameter("script");
         GroovyCodeSource gcs = new GroovyCodeSource(script, "test", "Test");
-        shell.evaluate(gcs);
+        shell.evaluate(gcs); // BAD: Groovy code injection
     }
 }
 

java/ql/src/Security/CWE/CWE-094/InstallApkWithFile.java

@@ -9,6 +9,7 @@
 File file = new File(Environment.getExternalStorageDirectory(), "myapp.apk");
 Intent intent = new Intent(Intent.ACTION_VIEW);
 /* Set the mimetype to APK */
+// BAD: The file may be altered by another app
 intent.setDataAndType(Uri.fromFile(file), "application/vnd.android.package-archive");
 
 startActivity(intent);

java/ql/src/Security/CWE/CWE-094/InstallApkWithFileProvider.java

@@ -21,6 +21,7 @@
 
 /* Expose temporary file with FileProvider */
 File toInstall = new File(this.getFilesDir(), tempFilename);
+// GOOD: The file is protected by FileProvider
 Uri applicationUri = FileProvider.getUriForFile(this, "com.example.apkprovider", toInstall);
 
 /* Create Intent and set data to APK file. */

java/ql/src/Security/CWE/CWE-094/InstallApkWithPackageInstaller.java

@@ -1,3 +1,4 @@
+// GOOD: Package installed using PackageInstaller
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageInstaller;