diff --git a/java/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.java b/java/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.java index 65735194a69..112519f7dda 100644 --- a/java/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.java +++ b/java/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.java @@ -4,6 +4,7 @@ public class SQLInjection extends HttpServlet { 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("'"); diff --git a/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalBad.java b/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalBad.java index 961d8c21c6b..98985647332 100644 --- a/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalBad.java +++ b/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalBad.java @@ -1,6 +1,6 @@ public class PartialPathTraversalBad { public void example(File dir, File parent) throws IOException { - if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath())) { + if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath())) { // BAD: dir.getCanonicalPath() not slash-terminated throw new IOException("Path traversal attempt: " + dir.getCanonicalPath()); } } diff --git a/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalGood.java b/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalGood.java index 65392373a25..2ad78d9d39e 100644 --- a/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalGood.java +++ b/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalGood.java @@ -2,7 +2,7 @@ import java.io.File; public class PartialPathTraversalGood { public void example(File dir, File parent) throws IOException { - if (!dir.toPath().normalize().startsWith(parent.toPath())) { + if (!dir.toPath().normalize().startsWith(parent.toPath())) { // GOOD: Check if dir.Path() is normalised throw new IOException("Path traversal attempt: " + dir.getCanonicalPath()); } } diff --git a/java/ql/src/Security/CWE/CWE-079/AndroidWebViewAddJavascriptInterfaceExample.java b/java/ql/src/Security/CWE/CWE-079/AndroidWebViewAddJavascriptInterfaceExample.java index fb4e1182a5a..5beeb505000 100644 --- a/java/ql/src/Security/CWE/CWE-079/AndroidWebViewAddJavascriptInterfaceExample.java +++ b/java/ql/src/Security/CWE/CWE-079/AndroidWebViewAddJavascriptInterfaceExample.java @@ -20,4 +20,4 @@ webview.addJavaScriptInterface(new ExposedObject(), "exposedObject"); webview.loadData("", "text/html", null); String name = "Robert'; DROP TABLE students; --"; -webview.loadUrl("javascript:alert(exposedObject.studentEmail(\""+ name +"\"))"); +webview.loadUrl("javascript:alert(exposedObject.studentEmail(\""+ name +"\"))"); // BAD: Untrusted input loaded into WebView diff --git a/java/ql/src/Security/CWE/CWE-079/WebSettingsDisableJavascript.java b/java/ql/src/Security/CWE/CWE-079/WebSettingsDisableJavascript.java index 43a8cd92c0e..13b83848135 100644 --- a/java/ql/src/Security/CWE/CWE-079/WebSettingsDisableJavascript.java +++ b/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 diff --git a/java/ql/src/Security/CWE/CWE-079/WebSettingsEnableJavascript.java b/java/ql/src/Security/CWE/CWE-079/WebSettingsEnableJavascript.java index adfac156009..c75c4c0a2b1 100644 --- a/java/ql/src/Security/CWE/CWE-079/WebSettingsEnableJavascript.java +++ b/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 diff --git a/java/ql/src/Security/CWE/CWE-094/GroovyInjectionBad.java b/java/ql/src/Security/CWE/CWE-094/GroovyInjectionBad.java index 8afe77d2a39..2111d11c9a5 100644 --- a/java/ql/src/Security/CWE/CWE-094/GroovyInjectionBad.java +++ b/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 } } diff --git a/java/ql/src/Security/CWE/CWE-094/InstallApkWithFile.java b/java/ql/src/Security/CWE/CWE-094/InstallApkWithFile.java index 39462a43cc9..bbaaa53e8e3 100644 --- a/java/ql/src/Security/CWE/CWE-094/InstallApkWithFile.java +++ b/java/ql/src/Security/CWE/CWE-094/InstallApkWithFile.java @@ -9,6 +9,6 @@ import java.io.File; File file = new File(Environment.getExternalStorageDirectory(), "myapp.apk"); Intent intent = new Intent(Intent.ACTION_VIEW); /* Set the mimetype to APK */ -intent.setDataAndType(Uri.fromFile(file), "application/vnd.android.package-archive"); +intent.setDataAndType(Uri.fromFile(file), "application/vnd.android.package-archive"); // BAD: The file may be altered by another app startActivity(intent); diff --git a/java/ql/src/Security/CWE/CWE-094/InstallApkWithFileProvider.java b/java/ql/src/Security/CWE/CWE-094/InstallApkWithFileProvider.java index d6e537caf7d..600eaa7dbf9 100644 --- a/java/ql/src/Security/CWE/CWE-094/InstallApkWithFileProvider.java +++ b/java/ql/src/Security/CWE/CWE-094/InstallApkWithFileProvider.java @@ -21,7 +21,7 @@ try (InputStream is = getAssets().open(assetName); /* Expose temporary file with FileProvider */ File toInstall = new File(this.getFilesDir(), tempFilename); -Uri applicationUri = FileProvider.getUriForFile(this, "com.example.apkprovider", toInstall); +Uri applicationUri = FileProvider.getUriForFile(this, "com.example.apkprovider", toInstall); // GOOD: The file is protected by FileProvider /* Create Intent and set data to APK file. */ Intent intent = new Intent(Intent.ACTION_INSTALL_PACKAGE); diff --git a/java/ql/src/Security/CWE/CWE-094/InstallApkWithPackageInstaller.java b/java/ql/src/Security/CWE/CWE-094/InstallApkWithPackageInstaller.java index fbe8eca3f47..909493b47b1 100644 --- a/java/ql/src/Security/CWE/CWE-094/InstallApkWithPackageInstaller.java +++ b/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; diff --git a/java/ql/src/Security/CWE/CWE-094/SSTIBad.java b/java/ql/src/Security/CWE/CWE-094/SSTIBad.java index 33210448e18..d1ad7e35e26 100644 --- a/java/ql/src/Security/CWE/CWE-094/SSTIBad.java +++ b/java/ql/src/Security/CWE/CWE-094/SSTIBad.java @@ -14,6 +14,6 @@ public class VelocitySSTI { StringWriter w = new StringWriter(); // evaluate( Context context, Writer out, String logTag, String instring ) - Velocity.evaluate(context, w, "mystring", code); + Velocity.evaluate(context, w, "mystring", code); // BAD: code is controlled by the user } } diff --git a/java/ql/src/Security/CWE/CWE-094/SSTIGood.java b/java/ql/src/Security/CWE/CWE-094/SSTIGood.java index be4120539d5..288dd525901 100644 --- a/java/ql/src/Security/CWE/CWE-094/SSTIGood.java +++ b/java/ql/src/Security/CWE/CWE-094/SSTIGood.java @@ -11,7 +11,7 @@ public class VelocitySSTI { String s = "We are using $project $name to render this."; StringWriter w = new StringWriter(); - Velocity.evaluate(context, w, "mystring", s); + Velocity.evaluate(context, w, "mystring", s); // GOOD: s is a constant string System.out.println(" string : " + w); } } diff --git a/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithSandbox.java b/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithSandbox.java index 43b3acfb65e..25cc0193198 100644 --- a/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithSandbox.java +++ b/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithSandbox.java @@ -4,7 +4,7 @@ public void evaluate(Socket socket) throws IOException { JexlSandbox onlyMath = new JexlSandbox(false); onlyMath.white("java.lang.Math"); - JexlEngine jexl = new JexlBuilder().sandbox(onlyMath).create(); + JexlEngine jexl = new JexlBuilder().sandbox(onlyMath).create(); // GOOD: using a sandbox String input = reader.readLine(); JexlExpression expression = jexl.createExpression(input); diff --git a/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithUberspectSandbox.java b/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithUberspectSandbox.java index 5952668b8b3..8515083b0ec 100644 --- a/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithUberspectSandbox.java +++ b/java/ql/src/Security/CWE/CWE-094/SaferJexlExpressionEvaluationWithUberspectSandbox.java @@ -6,7 +6,7 @@ public void evaluate(Socket socket) throws IOException { JexlEngine jexl = new JexlBuilder().uberspect(sandbox).create(); String input = reader.readLine(); - JexlExpression expression = jexl.createExpression(input); + JexlExpression expression = jexl.createExpression(input); // GOOD: jexl uses a sandbox JexlContext context = new MapContext(); expression.evaluate(context); } diff --git a/java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java b/java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java index 04dc4a5220f..438c48bcfc0 100644 --- a/java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java +++ b/java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java @@ -4,9 +4,9 @@ public Object evaluate(Socket socket) throws IOException { String string = reader.readLine(); ExpressionParser parser = new SpelExpressionParser(); - Expression expression = parser.parseExpression(string); + Expression expression = parser.parseExpression(string); // AVOID: string is controlled by the user SimpleEvaluationContext context = SimpleEvaluationContext.forReadWriteDataBinding().build(); - return expression.getValue(context); + return expression.getValue(context); // OK: Untrusted expressions are evaluated in a restricted context } } \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-094/UnsafeJexlExpressionEvaluation.java b/java/ql/src/Security/CWE/CWE-094/UnsafeJexlExpressionEvaluation.java index 986fa9833cb..762dfdf2f31 100644 --- a/java/ql/src/Security/CWE/CWE-094/UnsafeJexlExpressionEvaluation.java +++ b/java/ql/src/Security/CWE/CWE-094/UnsafeJexlExpressionEvaluation.java @@ -4,7 +4,7 @@ public void evaluate(Socket socket) throws IOException { String input = reader.readLine(); JexlEngine jexl = new JexlBuilder().create(); - JexlExpression expression = jexl.createExpression(input); + JexlExpression expression = jexl.createExpression(input); // BAD: input is controlled by the user JexlContext context = new MapContext(); expression.evaluate(context); } diff --git a/java/ql/src/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java b/java/ql/src/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java index b16a1eb50d2..d5b8060d2d3 100644 --- a/java/ql/src/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java +++ b/java/ql/src/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java @@ -4,7 +4,7 @@ public Object evaluate(Socket socket) throws IOException { String string = reader.readLine(); ExpressionParser parser = new SpelExpressionParser(); - Expression expression = parser.parseExpression(string); + Expression expression = parser.parseExpression(string); // BAD: string is controlled by the user return expression.getValue(); } } \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-1204/BadStaticInitializationVector.java b/java/ql/src/Security/CWE/CWE-1204/BadStaticInitializationVector.java index 85e8be6d8ce..b2adaa09e15 100644 --- a/java/ql/src/Security/CWE/CWE-1204/BadStaticInitializationVector.java +++ b/java/ql/src/Security/CWE/CWE-1204/BadStaticInitializationVector.java @@ -1,4 +1,4 @@ -byte[] iv = new byte[16]; // all zeroes +byte[] iv = new byte[16]; // BAD: all zeroes GCMParameterSpec params = new GCMParameterSpec(128, iv); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); cipher.init(Cipher.ENCRYPT_MODE, key, params); \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-1204/GoodRandomInitializationVector.java b/java/ql/src/Security/CWE/CWE-1204/GoodRandomInitializationVector.java index faceb119d64..fa1b0b40739 100644 --- a/java/ql/src/Security/CWE/CWE-1204/GoodRandomInitializationVector.java +++ b/java/ql/src/Security/CWE/CWE-1204/GoodRandomInitializationVector.java @@ -1,6 +1,6 @@ byte[] iv = new byte[16]; SecureRandom random = SecureRandom.getInstanceStrong(); -random.nextBytes(iv); +random.nextBytes(iv); // GOOD: random initialization vector GCMParameterSpec params = new GCMParameterSpec(128, iv); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); cipher.init(Cipher.ENCRYPT_MODE, key, params); \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextBad.java b/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextBad.java index d94e667b3db..73b09395597 100644 --- a/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextBad.java +++ b/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextBad.java @@ -1,2 +1,2 @@ TextView pwView = getViewById(R.id.pw_text); -pwView.setText("Your password is: " + password); \ No newline at end of file +pwView.setText("Your password is: " + password); // BAD: password is shown immediately \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextGood.java b/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextGood.java index 507fdae731c..93c7aeabdfc 100644 --- a/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextGood.java +++ b/java/ql/src/Security/CWE/CWE-200/AndroidSensitiveTextGood.java @@ -5,6 +5,6 @@ pwView.setText("Your password is: " + password); Button showButton = findViewById(R.id.show_pw_button); showButton.setOnClickListener(new View.OnClickListener() { public void onClick(View v) { - pwView.setVisibility(View.VISIBLE); + pwView.setVisibility(View.VISIBLE); // GOOD: password is only shown when the user clicks the button } }); diff --git a/java/ql/src/Security/CWE/CWE-200/ContentAccessDisabled.java b/java/ql/src/Security/CWE/CWE-200/ContentAccessDisabled.java index 25214a69afe..4435d411374 100644 --- a/java/ql/src/Security/CWE/CWE-200/ContentAccessDisabled.java +++ b/java/ql/src/Security/CWE/CWE-200/ContentAccessDisabled.java @@ -1,3 +1,4 @@ WebSettings settings = webview.getSettings(); +// GOOD: WebView is configured to disallow content access settings.setAllowContentAccess(false); diff --git a/java/ql/src/Security/CWE/CWE-200/ContentAccessEnabled.java b/java/ql/src/Security/CWE/CWE-200/ContentAccessEnabled.java index 3e5e6cb466b..13f90730f59 100644 --- a/java/ql/src/Security/CWE/CWE-200/ContentAccessEnabled.java +++ b/java/ql/src/Security/CWE/CWE-200/ContentAccessEnabled.java @@ -1,3 +1,4 @@ WebSettings settings = webview.getSettings(); +// BAD: WebView is configured to allow content access settings.setAllowContentAccess(true); diff --git a/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessSafe.java b/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessSafe.java index 6002888cba1..e5217d92f86 100644 --- a/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessSafe.java +++ b/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessSafe.java @@ -1,5 +1,6 @@ WebSettings settings = view.getSettings(); +// GOOD: WebView is configured to disallow file access settings.setAllowFileAccess(false); settings.setAllowFileAccessFromURLs(false); settings.setAllowUniversalAccessFromURLs(false); diff --git a/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessUnsafe.java b/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessUnsafe.java index 6c17d66c3b0..c2496793376 100644 --- a/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessUnsafe.java +++ b/java/ql/src/Security/CWE/CWE-200/WebViewFileAccessUnsafe.java @@ -1,5 +1,6 @@ WebSettings settings = view.getSettings(); +// BAD: WebView is configured to allow file access settings.setAllowFileAccess(true); settings.setAllowFileAccessFromURLs(true); settings.setAllowUniversalAccessFromURLs(true); diff --git a/java/ql/src/Security/CWE/CWE-330/examples/InsecureRandomnessCookie.java b/java/ql/src/Security/CWE/CWE-330/examples/InsecureRandomnessCookie.java index 151f5cddc29..f3e952b4f0f 100644 --- a/java/ql/src/Security/CWE/CWE-330/examples/InsecureRandomnessCookie.java +++ b/java/ql/src/Security/CWE/CWE-330/examples/InsecureRandomnessCookie.java @@ -1,4 +1,4 @@ -Random r = new Random(); +Random r = new Random(); // BAD: Random is not cryptographically secure byte[] bytes = new byte[16]; r.nextBytes(bytes); diff --git a/java/ql/src/Security/CWE/CWE-330/examples/SecureRandomnessCookie.java b/java/ql/src/Security/CWE/CWE-330/examples/SecureRandomnessCookie.java index 62395a7f086..fbe3b722434 100644 --- a/java/ql/src/Security/CWE/CWE-330/examples/SecureRandomnessCookie.java +++ b/java/ql/src/Security/CWE/CWE-330/examples/SecureRandomnessCookie.java @@ -1,4 +1,4 @@ -SecureRandom r = new SecureRandom(); +SecureRandom r = new SecureRandom(); // GOOD: SecureRandom is cryptographically secure byte[] bytes = new byte[16]; r.nextBytes(bytes); diff --git a/java/ql/src/Security/CWE/CWE-367/TOCTOURace.java b/java/ql/src/Security/CWE/CWE-367/TOCTOURace.java index 85a49ec9b04..aa08179f98d 100644 --- a/java/ql/src/Security/CWE/CWE-367/TOCTOURace.java +++ b/java/ql/src/Security/CWE/CWE-367/TOCTOURace.java @@ -12,14 +12,14 @@ class Resource { public synchronized void bad(Resource r) { if (r.isReady()) { - // r might no longer be ready, another thread might + // BAD: r might no longer be ready, another thread might // have called setReady(false) r.act(); } } public synchronized void good(Resource r) { - synchronized(r) { + synchronized(r) { // GOOD: r is locked if (r.isReady()) { r.act(); } diff --git a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationBad.java b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationBad.java index d5a37b7bf0f..8f05b2ffb61 100644 --- a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationBad.java +++ b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationBad.java @@ -7,6 +7,6 @@ public MyObject { public MyObject deserialize(Socket sock) { try(ObjectInputStream in = new ObjectInputStream(sock.getInputStream())) { - return (MyObject)in.readObject(); // unsafe + return (MyObject)in.readObject(); // BAD: in is from untrusted source } } diff --git a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationGood.java b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationGood.java index 4a1d5e6c3cd..abf5486d576 100644 --- a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationGood.java +++ b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserializationGood.java @@ -1,5 +1,5 @@ public MyObject deserialize(Socket sock) { try(DataInputStream in = new DataInputStream(sock.getInputStream())) { - return new MyObject(in.readInt()); + return new MyObject(in.readInt()); // GOOD: read only an int } } diff --git a/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdap.java b/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdap.java index 36826bf27b0..dddcea17a2c 100644 --- a/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdap.java +++ b/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdap.java @@ -1,3 +1,4 @@ +// BAD: LDAP authentication is used String ldapUrl = "ldap://ad.your-server.com:389"; Hashtable environment = new Hashtable(); environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); diff --git a/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdaps.java b/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdaps.java index e7d8bdefc60..bdefbe79d6c 100644 --- a/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdaps.java +++ b/java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdaps.java @@ -1,3 +1,4 @@ +// GOOD: LDAP connection using LDAPS String ldapUrl = "ldaps://ad.your-server.com:636"; Hashtable environment = new Hashtable(); environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); diff --git a/java/ql/src/Security/CWE/CWE-522/LdapEnableSasl.java b/java/ql/src/Security/CWE/CWE-522/LdapEnableSasl.java index 2e191c62918..71568c40791 100644 --- a/java/ql/src/Security/CWE/CWE-522/LdapEnableSasl.java +++ b/java/ql/src/Security/CWE/CWE-522/LdapEnableSasl.java @@ -1,3 +1,4 @@ +// GOOD: LDAP is used but SASL authentication is enabled String ldapUrl = "ldap://ad.your-server.com:389"; Hashtable environment = new Hashtable(); environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); diff --git a/java/ql/src/Security/CWE/CWE-611/XXEBad.java b/java/ql/src/Security/CWE/CWE-611/XXEBad.java index 7b7baeadfda..0049b254507 100644 --- a/java/ql/src/Security/CWE/CWE-611/XXEBad.java +++ b/java/ql/src/Security/CWE/CWE-611/XXEBad.java @@ -1,5 +1,5 @@ public void parse(Socket sock) throws Exception { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); DocumentBuilder builder = factory.newDocumentBuilder(); - builder.parse(sock.getInputStream()); //unsafe + builder.parse(sock.getInputStream()); // BAD: DTD parsing is enabled } diff --git a/java/ql/src/Security/CWE/CWE-611/XXEGood.java b/java/ql/src/Security/CWE/CWE-611/XXEGood.java index f1eef22e62a..91c28448299 100644 --- a/java/ql/src/Security/CWE/CWE-611/XXEGood.java +++ b/java/ql/src/Security/CWE/CWE-611/XXEGood.java @@ -2,5 +2,5 @@ public void disableDTDParse(Socket sock) throws Exception { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); DocumentBuilder builder = factory.newDocumentBuilder(); - builder.parse(sock.getInputStream()); //safe + builder.parse(sock.getInputStream()); // GOOD: DTD parsing is disabled } diff --git a/java/ql/src/Security/CWE/CWE-798/HardcodedAWSCredentials.java b/java/ql/src/Security/CWE/CWE-798/HardcodedAWSCredentials.java index 7528311bc4b..3c94ac4e833 100644 --- a/java/ql/src/Security/CWE/CWE-798/HardcodedAWSCredentials.java +++ b/java/ql/src/Security/CWE/CWE-798/HardcodedAWSCredentials.java @@ -3,7 +3,7 @@ import com.amazonaws.auth.BasicAWSCredentials; public class HardcodedAWSCredentials { public static void main(String[] args) { - //Hardcoded credentials for connecting to AWS services + // BAD: Hardcoded credentials for connecting to AWS services //To fix the problem, use other approaches including AWS credentials file, environment variables, or instance/container credentials instead AWSCredentials creds = new BasicAWSCredentials("ACCESS_KEY", "SECRET_KEY"); //sensitive call } diff --git a/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.java b/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.java index a12a9881d1a..5e996abe89d 100644 --- a/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.java +++ b/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.java @@ -1,8 +1,8 @@ -private static final String p = "123456"; // hard-coded credential +private static final String p = "123456"; // BAD: hard-coded credential public static void main(String[] args) throws SQLException { String url = "jdbc:mysql://localhost/test"; - String u = "admin"; // hard-coded credential + String u = "admin"; // BAD: hard-coded credential getConn(url, u, p); } diff --git a/java/ql/src/Security/CWE/CWE-835/InfiniteLoopBad.java b/java/ql/src/Security/CWE/CWE-835/InfiniteLoopBad.java index 69e13801c22..f0f408f5c45 100644 --- a/java/ql/src/Security/CWE/CWE-835/InfiniteLoopBad.java +++ b/java/ql/src/Security/CWE/CWE-835/InfiniteLoopBad.java @@ -1,5 +1,5 @@ for (int i=0; i<10; i++) { - for (int j=0; i<10; j++) { + for (int j=0; i<10; j++) { // BAD: Potential infinite loop: i should be j // do stuff if (shouldBreak()) break; } diff --git a/java/ql/src/Security/CWE/CWE-835/InfiniteLoopGood.java b/java/ql/src/Security/CWE/CWE-835/InfiniteLoopGood.java index 06c18c4e68b..01050b38ef3 100644 --- a/java/ql/src/Security/CWE/CWE-835/InfiniteLoopGood.java +++ b/java/ql/src/Security/CWE/CWE-835/InfiniteLoopGood.java @@ -1,5 +1,5 @@ for (int i=0; i<10; i++) { - for (int j=0; j<10; j++) { + for (int j=0; j<10; j++) { // GOOD: correct variable j // do stuff if (shouldBreak()) break; } diff --git a/java/ql/src/Security/CWE/CWE-925/Bad.java b/java/ql/src/Security/CWE/CWE-925/Bad.java index 376805f824e..1ab7bd38166 100644 --- a/java/ql/src/Security/CWE/CWE-925/Bad.java +++ b/java/ql/src/Security/CWE/CWE-925/Bad.java @@ -1,6 +1,7 @@ public class ShutdownReceiver extends BroadcastReceiver { @Override public void onReceive(final Context context, final Intent intent) { + // BAD: The code does not check if the intent is an ACTION_SHUTDOWN intent mainActivity.saveLocalData(); mainActivity.stopActivity(); } diff --git a/java/ql/src/Security/CWE/CWE-925/Good.java b/java/ql/src/Security/CWE/CWE-925/Good.java index b6ad1c43193..0414d4c198e 100644 --- a/java/ql/src/Security/CWE/CWE-925/Good.java +++ b/java/ql/src/Security/CWE/CWE-925/Good.java @@ -1,6 +1,7 @@ public class ShutdownReceiver extends BroadcastReceiver { @Override public void onReceive(final Context context, final Intent intent) { + // GOOD: The code checks if the intent is an ACTION_SHUTDOWN intent if (!intent.getAction().equals(Intent.ACTION_SHUTDOWN)) { return; }