From f0604e2e8472697d64d7a42d21b6441f8a48bf27 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 3 Sep 2021 12:59:03 +0200 Subject: [PATCH 01/13] Added query for Cleartext Storage in Android Database --- .../code/java/dataflow/ExternalFlow.qll | 2 + .../frameworks/android/ContentProviders.qll | 23 +++ .../code/java/frameworks/android/SQLite.qll | 8 + .../code/java/frameworks/android/Widget.qll | 21 +++ .../CleartextStorageAndroidDatabaseQuery.qll | 119 +++++++++++++++ .../code/java/security/SensitiveActions.qll | 10 +- .../CleartextStorageAndroidDatabase.java | 18 +++ .../CleartextStorageAndroidDatabase.qhelp | 36 +++++ .../CleartextStorageAndroidDatabase.ql | 23 +++ ...-08-17-cleartext-storage-database-query.md | 4 + ...eartextStorageAndroidDatabaseTest.expected | 0 .../CleartextStorageAndroidDatabaseTest.java | 138 ++++++++++++++++++ .../CleartextStorageAndroidDatabaseTest.ql | 22 +++ .../android/annotation/IntRange.java | 21 +++ .../android/content/Context.java | 1 + .../android/content/Intent.java | 1 + .../android/database/DatabaseUtils.java | 46 ++++++ .../android/database/SQLException.java | 29 ++++ .../database/sqlite/SQLiteDatabase.java | 1 + .../database/sqlite/SQLiteException.java | 30 ++++ .../database/sqlite/SQLiteQueryBuilder.java | 57 ++++++++ .../android/os/Bundle.java | 1 + .../android/webkit/WebResourceResponse.java | 1 - .../android/webkit/WebSettings.java | 5 +- .../android/webkit/WebView.java | 1 + .../android/webkit/WebViewClient.java | 3 - 26 files changed, 611 insertions(+), 10 deletions(-) create mode 100644 java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll create mode 100644 java/ql/lib/semmle/code/java/frameworks/android/Widget.qll create mode 100644 java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll create mode 100644 java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java create mode 100644 java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp create mode 100644 java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql create mode 100644 java/ql/src/change-notes/2021-08-17-cleartext-storage-database-query.md create mode 100644 java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.expected create mode 100644 java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java create mode 100644 java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql create mode 100644 java/ql/test/stubs/google-android-9.0.0/android/annotation/IntRange.java create mode 100644 java/ql/test/stubs/google-android-9.0.0/android/database/DatabaseUtils.java create mode 100644 java/ql/test/stubs/google-android-9.0.0/android/database/SQLException.java create mode 100644 java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteException.java create mode 100644 java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteQueryBuilder.java diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index 839d1fa69e28..ab7404d10119 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -132,6 +132,8 @@ private module Frameworks { private import semmle.code.java.frameworks.Hibernate private import semmle.code.java.frameworks.jOOQ private import semmle.code.java.frameworks.spring.SpringHttp + private import semmle.code.java.frameworks.android.ContentProviders + private import semmle.code.java.frameworks.android.Widget } private predicate sourceModelCsv(string row) { diff --git a/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll b/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll new file mode 100644 index 000000000000..abda2af3c035 --- /dev/null +++ b/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll @@ -0,0 +1,23 @@ +/** + * Provides classes and predicates for working with Content Providers. + */ + +import java +import semmle.code.java.dataflow.ExternalFlow + +/** The class `android.content.ContentValues`. */ +class ContentValues extends Class { + ContentValues() { this.hasQualifiedName("android.content", "ContentValues") } +} + +private class SummaryModels extends SummaryModelCsv { + override predicate row(string row) { + row = + [ + "android.content;ContentValues;false;put;;;Argument[0];MapKey of Argument[-1];value", + "android.content;ContentValues;false;put;;;Argument[1];MapValue of Argument[-1];value", + "android.content;ContentValues;false;putAll;;;MapKey of Argument[0];MapKey of Argument[-1];value", + "android.content;ContentValues;false;putAll;;;MapValue of Argument[0];MapValue of Argument[-1];value" + ] + } +} diff --git a/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll index 1d189cca5699..5c1a9d6e600f 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll @@ -24,6 +24,14 @@ class TypeDatabaseUtils extends Class { TypeDatabaseUtils() { hasQualifiedName("android.database", "DatabaseUtils") } } +class TypeSQLiteOpenHelper extends Class { + TypeSQLiteOpenHelper() { this.hasQualifiedName("android.database.sqlite", "SQLiteOpenHelper") } +} + +class TypeSQLiteStatement extends Class { + TypeSQLiteStatement() { this.hasQualifiedName("android.database.sqlite", "SQLiteStatement") } +} + private class SQLiteSinkCsv extends SinkModelCsv { override predicate row(string row) { row = diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll b/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll new file mode 100644 index 000000000000..f075e758b319 --- /dev/null +++ b/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll @@ -0,0 +1,21 @@ +import java +import semmle.code.java.dataflow.ExternalFlow +import semmle.code.java.dataflow.FlowSources + +private class AndroidWidgetSourceModels extends SourceModelCsv { + override predicate row(string row) { + row = ["android.widget;EditText;true;getText;;;ReturnValue;android-widget"] + } +} + +private class DefaultAndroidWidgetSources extends RemoteFlowSource { + DefaultAndroidWidgetSources() { sourceNode(this, "android-widget") } + + override string getSourceType() { result = "Android widget source" } +} + +private class AndroidWidgetSummaryModels extends SummaryModelCsv { + override predicate row(string row) { + row = ["android.widget;EditText;true;getText;;;Argument[-1];ReturnValue;taint"] + } +} diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll new file mode 100644 index 000000000000..327d26a7ebb1 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -0,0 +1,119 @@ +/** Provides classes and predicates to reason about cleartext storage in Android databases. */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.frameworks.android.ContentProviders +import semmle.code.java.frameworks.android.Intent +import semmle.code.java.frameworks.android.SQLite +import semmle.code.java.security.CleartextStorageQuery + +private class LocalDatabaseCleartextStorageSink extends CleartextStorageSink { + LocalDatabaseCleartextStorageSink() { localDatabaseInput(_, this.asExpr()) } +} + +private class LocalDatabaseCleartextStorageStep extends CleartextStorageAdditionalTaintStep { + override predicate step(DataFlow::Node n1, DataFlow::Node n2) { + // EditText.getText() return type is parsed as `Object`, so we need to + // add a taint step for `Object.toString` to model `editText.getText().toString()` + exists(MethodAccess ma, Method m | + ma.getMethod() = m and + m.getDeclaringType() instanceof TypeObject and + m.hasName("toString") + | + n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma + ) + } +} + +/** The creation of an object that can be used to store data in a local database. */ +class LocalDatabaseOpenMethodAccess extends Storable, Call { + LocalDatabaseOpenMethodAccess() { + exists(Method m | this.(MethodAccess).getMethod() = m | + m.getDeclaringType().getASupertype*() instanceof TypeSQLiteOpenHelper and + m.hasName("getWritableDatabase") + or + m.getDeclaringType() instanceof TypeSQLiteDatabase and + m.hasName(["create", "open%Database", "compileStatement"]) + or + m.getDeclaringType().getASupertype*() instanceof TypeContext and + m.hasName("openOrCreateDatabase") + ) + or + this.(ClassInstanceExpr).getConstructedType() instanceof ContentValues + } + + override Expr getAnInput() { + exists(LocalDatabaseFlowConfig config, DataFlow::Node database | + localDatabaseInput(database, result) and + config.hasFlow(DataFlow::exprNode(this), database) + ) + } + + override Expr getAStore() { + exists(LocalDatabaseFlowConfig config, DataFlow::Node database | + localDatabaseStore(database, result) and + config.hasFlow(DataFlow::exprNode(this), database) + ) + } +} + +/** A method that is both a database input and a database store. */ +private class LocalDatabaseInputStoreMethod extends Method { + LocalDatabaseInputStoreMethod() { + this.getDeclaringType() instanceof TypeSQLiteDatabase and + this.getName().matches("exec%SQL") + } +} + +private predicate localDatabaseInput(DataFlow::Node database, Argument input) { + exists(Method m | input.getCall().(MethodAccess).getMethod() = m | + m instanceof LocalDatabaseInputStoreMethod and + database.asExpr() = input.getCall().getQualifier() + or + m.getDeclaringType() instanceof TypeSQLiteDatabase and + m.hasName("compileStatement") and + database.asExpr() = input.getCall() + or + m.getDeclaringType() instanceof ContentValues and + m.hasName("put") and + input.getPosition() = 1 and + database.asExpr() = input.getCall().getQualifier() + ) +} + +private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store) { + exists(Method m | store.getMethod() = m | + m instanceof LocalDatabaseInputStoreMethod and + database.asExpr() = store.getQualifier() + or + m.getDeclaringType() instanceof TypeSQLiteDatabase and + m.getName().matches(["insert%", "replace%", "update%"]) and + database.asExpr() = store.getAnArgument() and + database.getType() instanceof ContentValues + or + m.getDeclaringType() instanceof TypeSQLiteStatement and + m.hasName(["executeInsert", "executeUpdateDelete"]) and + database.asExpr() = store.getQualifier() + ) +} + +private class LocalDatabaseFlowConfig extends DataFlow::Configuration { + LocalDatabaseFlowConfig() { this = "LocalDatabaseFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof LocalDatabaseOpenMethodAccess + } + + override predicate isSink(DataFlow::Node sink) { + localDatabaseInput(sink, _) or + localDatabaseStore(sink, _) + } + + override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + exists(Field f | + f.getType() instanceof TypeSQLiteDatabase and + f.getAnAssignedValue() = n1.asExpr() and + f = n2.asExpr().(FieldRead).getField() + ) + } +} diff --git a/java/ql/lib/semmle/code/java/security/SensitiveActions.qll b/java/ql/lib/semmle/code/java/security/SensitiveActions.qll index c0c45975d8b8..05dcccc36cef 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveActions.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveActions.qll @@ -14,13 +14,19 @@ import java private string suspicious() { - result = ["%password%", "%passwd%", "%account%", "%accnt%", "%trusted%"] + result = + [ + "%password%", "%passwd%", "pwd", "%account%", "%accnt%", "%trusted%", "%refresh%token%", + "%secret%token" + ] } private string nonSuspicious() { result = "%hashed%" or result = "%encrypted%" or - result = "%crypt%" + result = "%crypt%" or + result = "%create table%" or + result = "%drop table%" } /** diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java new file mode 100644 index 000000000000..c421e7f3de51 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java @@ -0,0 +1,18 @@ +public void sqliteStorageUnsafe(Context ctx, String name, String password) { + // BAD - sensitive information saved in cleartext. + SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); + db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password}); +} + +public void sqliteStorageSafe(Context ctx, String name, String password) { + // GOOD - sensitive information encrypted with a custom method. + SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); + db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, encrypt(password)}); +} + +public void sqlCipherStorageSafe(String name, String password, String databasePassword) { + // GOOD - sensitive information saved using SQLCipher. + net.sqlcipher.database.SQLiteDatabase db = + net.sqlcipher.database.SQLiteDatabase.openOrCreateDatabase("test", databasePassword, null); + db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password}); +} diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp new file mode 100644 index 000000000000..3e190aa6a479 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp @@ -0,0 +1,36 @@ + + + +

+ SQLite is a lightweight database engine commonly used in Android devices to store data. By itself, SQLite does not offer any encryption mechanism by default and stores all data in plaintext, which introduces a risk if sensitive data like credentials, authentication tokens or personal identifiable information (PII) are directly stored in a SQLite database. The information could be accessed by any process or user in rooted devices, or can be disclosed through chained vulnerabilities, like unexpected access to the private storage through exposed components. +

+ + + +

+ Use SQLCipher or similar libraries to add encryption capabilities to SQLite. Alternatively, encrypt sensitive data using cryptographicaly secure algorithms before storing it in the database. +

+ + + +

+ In the first example, sensitive user information is stored in cleartext. +

+ +

+ In the second and third examples, the code encrypts sensitive information before saving it to the database. +

+ + + + +
  • + Android Developers: + Work with data more securely +
    • +
  • + SQLCipher: + Android Application Integration +
    • +     +     diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql new file mode 100644 index 000000000000..88c1c6f856ce --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql @@ -0,0 +1,23 @@ +/** + * @name Cleartext storage of sensitive information using a local database on Android + * @description Cleartext Storage of Sensitive Information using + * a local database on Android allows access for users with root + * privileges or unexpected exposure from chained vulnerabilities. + * @kind problem + * @problem.severity warning + * @precision medium + * @id java/android/cleartext-storage-database + * @tags security + * external/cwe/cwe-312 + */ + +import java +import semmle.code.java.security.CleartextStorageAndroidDatabaseQuery + +from SensitiveSource data, LocalDatabaseOpenMethodAccess s, Expr input, Expr store +where + input = s.getAnInput() and + store = s.getAStore() and + data.flowsToCached(input) +select store, "SQLite database $@ containing $@ is stored $@. Data was added $@.", s, s.toString(), + data, "sensitive data", store, "here", input, "here" diff --git a/java/ql/src/change-notes/2021-08-17-cleartext-storage-database-query.md b/java/ql/src/change-notes/2021-08-17-cleartext-storage-database-query.md new file mode 100644 index 000000000000..ce71dca1a5fc --- /dev/null +++ b/java/ql/src/change-notes/2021-08-17-cleartext-storage-database-query.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* A new query "Cleartext storage of sensitive information using a local database on Android" (`java/android/cleartext-storage-database`) has been added. This query finds instances of sensitive data being stored in local databases without encryption, which may expose it to attackers or malicious applications. diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.expected b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java new file mode 100644 index 000000000000..0b4e317534c4 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java @@ -0,0 +1,138 @@ +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.Base64; +import android.app.Activity; +import android.content.ContentValues; +import android.content.Context; +import android.database.sqlite.SQLiteDatabase; +import android.database.sqlite.SQLiteStatement; + +public class CleartextStorageAndroidDatabaseTest extends Activity { + + public void testCleartextStorageAndroiDatabaseSafe1(Context ctx, String name, String password) { + SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); + db.execSQL("CREATE TABLE IF NOT EXISTS users(user VARCHAR, password VARCHAR);"); // Safe + } + + public void testCleartextStorageAndroiDatabaseSafe2(Context ctx, String name, String password) { + SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); + db.execSQL("DROP TABLE passwords;"); // Safe - no sensitive value being stored + } + + public void testCleartextStorageAndroiDatabase1(Context ctx, String name, String password) { + SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); + String query = "INSERT INTO users VALUES ('" + name + "', '" + password + "');"; + db.execSQL(query); // $ hasCleartextStorageAndroidDatabase + } + + public void testCleartextStorageAndroiDatabase2(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.create(null); + String query = "INSERT INTO users VALUES (?, ?)"; + db.execSQL(query, new String[] {name, password}); // $ hasCleartextStorageAndroidDatabase + } + + //@formatter:off + public void testCleartextStorageAndroiDatabase3(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.create(null); + String query = "INSERT INTO users VALUES (?, ?)"; + db.execPerConnectionSQL(query, new String[] {name, password}); // $ hasCleartextStorageAndroidDatabase + } + //@formatter:on + + public void testCleartextStorageAndroiDatabaseSafe3(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // Safe - ContentValues aren't added to any database + } + + public void testCleartextStorageAndroiDatabase4(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.insert("table", null, cv); + } + + public void testCleartextStorageAndroiDatabase5(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.insertOrThrow("table", null, cv); + } + + public void testCleartextStorageAndroiDatabase6(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.insertWithOnConflict("table", null, cv, 0); + } + + public void testCleartextStorageAndroiDatabase7(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.replace("table", null, cv); + } + + public void testCleartextStorageAndroiDatabase8(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.replaceOrThrow("table", null, cv); + } + + public void testCleartextStorageAndroiDatabase9(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.update("table", cv, "", new String[] {}); + } + + public void testCleartextStorageAndroiDatabase10(String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + ContentValues cv = new ContentValues(); + cv.put("username", name); + cv.put("password", password); // $ hasCleartextStorageAndroidDatabase + db.updateWithOnConflict("table", cv, "", new String[] {}, 0); + } + + public void testCleartextStorageAndroiDatabaseSafe4(SQLiteDatabase db, String name, + String password) { + String query = "INSERT INTO users VALUES ('" + name + "', '" + password + "');"; + SQLiteStatement stmt = db.compileStatement(query); // Safe - statement isn't executed + } + + public void testCleartextStorageAndroiDatabase11(SQLiteDatabase db, String name, + String password) { + String query = "INSERT INTO users VALUES ('" + name + "', '" + password + "');"; + SQLiteStatement stmt = db.compileStatement(query); // $ hasCleartextStorageAndroidDatabase + stmt.executeUpdateDelete(); + } + + public void testCleartextStorageAndroiDatabase12(SQLiteDatabase db, String name, + String password) { + String query = "INSERT INTO users VALUES ('" + name + "', '" + password + "');"; + SQLiteStatement stmt = db.compileStatement(query); // $ hasCleartextStorageAndroidDatabase + stmt.executeInsert(); + } + + public void testCleartextStorageAndroiDatabaseSafe5(String name, String password) + throws Exception { + SQLiteDatabase db = SQLiteDatabase.create(null); + String query = "INSERT INTO users VALUES (?, ?)"; + db.execSQL(query, new String[] {name, encrypt(password)}); // Safe + } + + private static String encrypt(String cleartext) throws Exception { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8)); + String encoded = Base64.getEncoder().encodeToString(hash); + return encoded; + } +} diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql new file mode 100644 index 000000000000..ea754b1a96e3 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql @@ -0,0 +1,22 @@ +import java +import semmle.code.java.security.CleartextStorageAndroidDatabaseQuery +import TestUtilities.InlineExpectationsTest + +class CleartextStorageAndroidDatabaseTest extends InlineExpectationsTest { + CleartextStorageAndroidDatabaseTest() { this = "CleartextStorageAndroidDatabaseTest" } + + override string getARelevantTag() { result = "hasCleartextStorageAndroidDatabase" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasCleartextStorageAndroidDatabase" and + exists(SensitiveSource data, LocalDatabaseOpenMethodAccess s, Expr input, Expr store | + input = s.getAnInput() and + store = s.getAStore() and + data.flowsToCached(input) + | + input.getLocation() = location and + element = input.toString() and + value = "" + ) + } +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/annotation/IntRange.java b/java/ql/test/stubs/google-android-9.0.0/android/annotation/IntRange.java new file mode 100644 index 000000000000..fdd1786ea5e8 --- /dev/null +++ b/java/ql/test/stubs/google-android-9.0.0/android/annotation/IntRange.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package android.annotation; + +public @interface IntRange { + long from() default Long.MIN_VALUE; + + long to() default Long.MAX_VALUE; + +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java b/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java index 08865e47a8e0..e95df1dcad54 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java @@ -101,6 +101,7 @@ public Context(){} public abstract String getSystemServiceName(Class p0); public abstract String[] databaseList(); public abstract String[] fileList(); +<<<<<<< HEAD public abstract boolean bindService(Intent p0, ServiceConnection p1, int p2); public abstract boolean bindServiceAsUser(Intent p0, ServiceConnection p1, int p2, UserHandle p3); public abstract boolean deleteDatabase(String p0); diff --git a/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java b/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java index ad2631b31490..4fd200f2556b 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java @@ -15,6 +15,7 @@ import android.os.Bundle; import android.os.Parcel; import android.os.Parcelable; +<<<<<<< HEAD import android.util.AttributeSet; import java.io.Serializable; import java.util.ArrayList; diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/DatabaseUtils.java b/java/ql/test/stubs/google-android-9.0.0/android/database/DatabaseUtils.java new file mode 100644 index 000000000000..0d0414c9fdfe --- /dev/null +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/DatabaseUtils.java @@ -0,0 +1,46 @@ +package android.database; + +import android.content.Context; +import android.database.sqlite.SQLiteDatabase; +import android.os.ParcelFileDescriptor; + +public class DatabaseUtils { + + public static ParcelFileDescriptor blobFileDescriptorForQuery(SQLiteDatabase db, String query, + String[] selectionArgs) { + return null; + } + + public static long longForQuery(SQLiteDatabase db, String query, String[] selectionArgs) { + return 0; + + } + + public static String stringForQuery(SQLiteDatabase db, String query, String[] selectionArgs) { + return null; + + } + + public static void createDbFromSqlStatements(Context context, String dbName, int dbVersion, String sqlStatements) { + + } + + public static int queryNumEntries(SQLiteDatabase db, String table, String selection) { + return 0; + + } + + public static int queryNumEntries(SQLiteDatabase db, String table, String selection, String[] selectionArgs) { + return 0; + + } + + public static String[] appendSelectionArgs(String[] originalValues, String[] newValues) { + return null; + } + + public static String concatenateWhere(String a, String b) { + return null; + } + +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/SQLException.java b/java/ql/test/stubs/google-android-9.0.0/android/database/SQLException.java new file mode 100644 index 000000000000..87da8071d5e5 --- /dev/null +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/SQLException.java @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2006 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.database; + +public class SQLException extends RuntimeException { + public SQLException() { + } + + public SQLException(String error) { + } + + public SQLException(String error, Throwable cause) { + } + +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java index 64b62e68d443..a698c7e952fe 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java @@ -1,3 +1,4 @@ +<<<<<<< HEAD // Generated automatically from android.database.sqlite.SQLiteDatabase for testing purposes package android.database.sqlite; diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteException.java b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteException.java new file mode 100644 index 000000000000..323251bc00b6 --- /dev/null +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteException.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2006 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.database.sqlite; +import android.database.SQLException; + +public class SQLiteException extends SQLException { + public SQLiteException() { + } + + public SQLiteException(String error) { + } + + public SQLiteException(String error, Throwable cause) { + } + +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteQueryBuilder.java b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteQueryBuilder.java new file mode 100644 index 000000000000..01b8942c6d72 --- /dev/null +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteQueryBuilder.java @@ -0,0 +1,57 @@ +package android.database.sqlite; + +import java.util.Map; +import java.util.Set; + +import android.content.ContentValues; +import android.os.CancellationSignal; + +public abstract class SQLiteQueryBuilder { + public abstract void delete(SQLiteDatabase db, String selection, String[] selectionArgs); + + public abstract void insert(SQLiteDatabase db, ContentValues values); + + public abstract void query(SQLiteDatabase db, String[] projectionIn, String selection, String[] selectionArgs, + String groupBy, String having, String sortOrder); + + public abstract void query(SQLiteDatabase db, String[] projectionIn, String selection, String[] selectionArgs, + String groupBy, String having, String sortOrder, String limit); + + public abstract void query(SQLiteDatabase db, String[] projectionIn, String selection, String[] selectionArgs, + String groupBy, String having, String sortOrder, String limit, CancellationSignal cancellationSignal); + + public abstract void update(SQLiteDatabase db, ContentValues values, String selection, String[] selectionArgs); + + public static String buildQueryString(boolean distinct, String tables, String[] columns, String where, + String groupBy, String having, String orderBy, String limit) { + return null; + } + + public abstract String buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, + String limit); + + public abstract String buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, + String having, String sortOrder, String limit); + + public abstract String buildUnionQuery(String[] subQueries, String sortOrder, String limit); + + public abstract String buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, + Set columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, + String selection, String[] selectionArgs, String groupBy, String having); + + public abstract String buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, + Set columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, + String selection, String groupBy, String having); + + public static void appendColumns(StringBuilder s, String[] columns) { + } + + public abstract void setProjectionMap(Map columnMap); + + public abstract void setTables(String inTables); + + public abstract void appendWhere(CharSequence inWhere); + + public abstract void appendWhereStandalone(CharSequence inWhere); + +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java b/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java index 4beb1cf5dee3..190aa81c80d5 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java @@ -12,6 +12,7 @@ import android.util.SparseArray; import java.io.Serializable; import java.util.ArrayList; +<<<<<<< HEAD public class Bundle extends BaseBundle implements Cloneable, Parcelable { diff --git a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebResourceResponse.java b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebResourceResponse.java index 0df042650462..1a2ff3cc1da9 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebResourceResponse.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebResourceResponse.java @@ -16,7 +16,6 @@ package android.webkit; import java.io.InputStream; -import java.io.StringBufferInputStream; import java.util.Map; /** diff --git a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebSettings.java b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebSettings.java index e6a7d5d70508..33c9a1b8a571 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebSettings.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebSettings.java @@ -15,11 +15,8 @@ */ package android.webkit; +import java.net.CookieManager; import android.content.Context; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; /** * Manages settings state for a WebView. When a WebView is first created, it diff --git a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebView.java b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebView.java index d9625b707717..3065a9df966a 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebView.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebView.java @@ -18,6 +18,7 @@ import android.view.View; public class WebView extends View { + public WebView(Context context) { super(context); } diff --git a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebViewClient.java b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebViewClient.java index 4b1bd58498b8..03a98480210e 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebViewClient.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/webkit/WebViewClient.java @@ -15,9 +15,6 @@ */ package android.webkit; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - public class WebViewClient { /** * Give the host application a chance to take over the control when a new url is From 16b61f78e6707365a1800b7dbde479129b2fa981 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 3 Sep 2021 14:10:15 +0200 Subject: [PATCH 02/13] Fix QLDocs and the qhelp example --- .../lib/semmle/code/java/frameworks/android/SQLite.qll | 8 ++++++++ .../lib/semmle/code/java/frameworks/android/Widget.qll | 2 ++ .../CWE/CWE-312/CleartextStorageAndroidDatabase.java | 9 +++++++++ 3 files changed, 19 insertions(+) diff --git a/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll index 5c1a9d6e600f..684df41ac56e 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/SQLite.qll @@ -1,3 +1,5 @@ +/** Provides classes and predicates for working with SQLite databases. */ + import java import Android import semmle.code.java.dataflow.FlowSteps @@ -24,10 +26,16 @@ class TypeDatabaseUtils extends Class { TypeDatabaseUtils() { hasQualifiedName("android.database", "DatabaseUtils") } } +/** + * The class `android.database.sqlite.SQLiteOpenHelper`. + */ class TypeSQLiteOpenHelper extends Class { TypeSQLiteOpenHelper() { this.hasQualifiedName("android.database.sqlite", "SQLiteOpenHelper") } } +/** + * The class `android.database.sqlite.SQLiteStatement`. + */ class TypeSQLiteStatement extends Class { TypeSQLiteStatement() { this.hasQualifiedName("android.database.sqlite", "SQLiteStatement") } } diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll b/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll index f075e758b319..961c63a4069c 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll @@ -1,3 +1,5 @@ +/** Provides classes and predicates for working with Android widgets. */ + import java import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.dataflow.FlowSources diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java index c421e7f3de51..0145125448f7 100644 --- a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.java @@ -16,3 +16,12 @@ public void sqlCipherStorageSafe(String name, String password, String databasePa net.sqlcipher.database.SQLiteDatabase.openOrCreateDatabase("test", databasePassword, null); db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password}); } + +private static String encrypt(String cleartext) { + // Use an encryption or strong hashing algorithm in the real world. + // The example below just returns a SHA-256 hash. + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8)); + String encoded = Base64.getEncoder().encodeToString(hash); + return encoded; +} \ No newline at end of file From ee84dae164d774eb07bf9bc0a4518e0f3b1c2922 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 11 Nov 2021 10:46:00 +0100 Subject: [PATCH 03/13] Fix predicate name --- .../src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql | 2 +- .../security/CWE-312/CleartextStorageAndroidDatabaseTest.ql | 2 +- .../android/database/sqlite/SQLiteDatabase.java | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql index 88c1c6f856ce..df394db8b4cc 100644 --- a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.ql @@ -18,6 +18,6 @@ from SensitiveSource data, LocalDatabaseOpenMethodAccess s, Expr input, Expr sto where input = s.getAnInput() and store = s.getAStore() and - data.flowsToCached(input) + data.flowsTo(input) select store, "SQLite database $@ containing $@ is stored $@. Data was added $@.", s, s.toString(), data, "sensitive data", store, "here", input, "here" diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql index ea754b1a96e3..421b3a408c40 100644 --- a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql +++ b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.ql @@ -12,7 +12,7 @@ class CleartextStorageAndroidDatabaseTest extends InlineExpectationsTest { exists(SensitiveSource data, LocalDatabaseOpenMethodAccess s, Expr input, Expr store | input = s.getAnInput() and store = s.getAStore() and - data.flowsToCached(input) + data.flowsTo(input) | input.getLocation() = location and element = input.toString() and diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java index a698c7e952fe..f2b43056026a 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java @@ -97,6 +97,7 @@ public void endTransaction(){} public void execPerConnectionSQL(String p0, Object[] p1){} public void execSQL(String p0){} public void execSQL(String p0, Object[] p1){} + public void execPerConnectionSQL (String p0, Object[] p1){} public void markTableSyncable(String p0, String p1){} public void markTableSyncable(String p0, String p1, String p2){} public void setCustomAggregateFunction(String p0, BinaryOperator p1){} From c5ed5fcaac9a91cb225dfda370fa766a388c9e11 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 12 Nov 2021 15:24:56 +0100 Subject: [PATCH 04/13] Apply suggestions from code review Co-authored-by: Ethan Palm <56270045+ethanpalm@users.noreply.github.com> --- .../CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp index 3e190aa6a479..f8e6183f36f9 100644 --- a/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.qhelp @@ -2,13 +2,13 @@

    - SQLite is a lightweight database engine commonly used in Android devices to store data. By itself, SQLite does not offer any encryption mechanism by default and stores all data in plaintext, which introduces a risk if sensitive data like credentials, authentication tokens or personal identifiable information (PII) are directly stored in a SQLite database. The information could be accessed by any process or user in rooted devices, or can be disclosed through chained vulnerabilities, like unexpected access to the private storage through exposed components. + SQLite is a lightweight database engine commonly used in Android devices to store data. By itself, SQLite does not offer any encryption mechanism by default and stores all data in cleartext, which introduces a risk if sensitive data like credentials, authentication tokens or personal identifiable information (PII) are directly stored in a SQLite database. The information could be accessed by any process or user in rooted devices, or can be disclosed through chained vulnerabilities, like unexpected access to the private storage through exposed components.

    - Use SQLCipher or similar libraries to add encryption capabilities to SQLite. Alternatively, encrypt sensitive data using cryptographicaly secure algorithms before storing it in the database. + Use SQLCipher or similar libraries to add encryption capabilities to SQLite. Alternatively, encrypt sensitive data using cryptographically secure algorithms before storing it in the database.

    From 4e4f619ae407058652e7d90aec488e05e8194e97 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 17 Jan 2022 09:58:28 +0100 Subject: [PATCH 05/13] Update java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll Co-authored-by: Chris Smowton --- .../code/java/security/CleartextStorageAndroidDatabaseQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index 327d26a7ebb1..4be762e014dd 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -66,7 +66,7 @@ private class LocalDatabaseInputStoreMethod extends Method { } private predicate localDatabaseInput(DataFlow::Node database, Argument input) { - exists(Method m | input.getCall().(MethodAccess).getMethod() = m | + exists(Method m | input.getCall().getCallee() = m | m instanceof LocalDatabaseInputStoreMethod and database.asExpr() = input.getCall().getQualifier() or From baa1f71a5330980f518e59346b59bb2f46aba2a4 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 17 Jan 2022 10:22:00 +0100 Subject: [PATCH 06/13] Add QLDoc --- .../CleartextStorageAndroidDatabaseQuery.qll | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index 4be762e014dd..e6273a18e8f6 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -65,6 +65,11 @@ private class LocalDatabaseInputStoreMethod extends Method { } } +/** + * Holds if `input` is a value being prepared for being stored into the SQLite dataabse `database`. + * This can be done using prepared statements, using the class `ContentValues`, or directly + * appending `input` to a SQL query. + */ private predicate localDatabaseInput(DataFlow::Node database, Argument input) { exists(Method m | input.getCall().getCallee() = m | m instanceof LocalDatabaseInputStoreMethod and @@ -81,6 +86,11 @@ private predicate localDatabaseInput(DataFlow::Node database, Argument input) { ) } +/** + * Holds if `store` is a method call for storing a previously appended input value, + * either through the use of prepared statements, via the `ContentValues` class, or + * directly executing a raw SQL query. + */ private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store) { exists(Method m | store.getMethod() = m | m instanceof LocalDatabaseInputStoreMethod and @@ -110,6 +120,8 @@ private class LocalDatabaseFlowConfig extends DataFlow::Configuration { } override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + // Adds a step for tracking databases through field flow, that is, a database is opened and + // assigned to a field, and then an input or store method is called on that field elsewhere. exists(Field f | f.getType() instanceof TypeSQLiteDatabase and f.getAnAssignedValue() = n1.asExpr() and From 5cf664411b2e48279cd82cf50e505808f41da3d3 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 17 Jan 2022 10:22:30 +0100 Subject: [PATCH 07/13] Remove unneeded nonSuspicious values --- java/ql/lib/semmle/code/java/security/SensitiveActions.qll | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/SensitiveActions.qll b/java/ql/lib/semmle/code/java/security/SensitiveActions.qll index 05dcccc36cef..ceaae6fb40ee 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveActions.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveActions.qll @@ -24,9 +24,7 @@ private string suspicious() { private string nonSuspicious() { result = "%hashed%" or result = "%encrypted%" or - result = "%crypt%" or - result = "%create table%" or - result = "%drop table%" + result = "%crypt%" } /** From 652a1d2dc2ea7c1dcc787cac5b4b0ad63c8e9f11 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 17 Jan 2022 12:09:18 +0100 Subject: [PATCH 08/13] Fix wrongly resolved rebase conflicts --- .../test/stubs/google-android-9.0.0/android/content/Context.java | 1 - .../test/stubs/google-android-9.0.0/android/content/Intent.java | 1 - .../android/database/sqlite/SQLiteDatabase.java | 1 - java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java | 1 - 4 files changed, 4 deletions(-) diff --git a/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java b/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java index e95df1dcad54..08865e47a8e0 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/content/Context.java @@ -101,7 +101,6 @@ public Context(){} public abstract String getSystemServiceName(Class p0); public abstract String[] databaseList(); public abstract String[] fileList(); -<<<<<<< HEAD public abstract boolean bindService(Intent p0, ServiceConnection p1, int p2); public abstract boolean bindServiceAsUser(Intent p0, ServiceConnection p1, int p2, UserHandle p3); public abstract boolean deleteDatabase(String p0); diff --git a/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java b/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java index 4fd200f2556b..ad2631b31490 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/content/Intent.java @@ -15,7 +15,6 @@ import android.os.Bundle; import android.os.Parcel; import android.os.Parcelable; -<<<<<<< HEAD import android.util.AttributeSet; import java.io.Serializable; import java.util.ArrayList; diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java index f2b43056026a..f6dd58334821 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java @@ -1,4 +1,3 @@ -<<<<<<< HEAD // Generated automatically from android.database.sqlite.SQLiteDatabase for testing purposes package android.database.sqlite; diff --git a/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java b/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java index 190aa81c80d5..4beb1cf5dee3 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/os/Bundle.java @@ -12,7 +12,6 @@ import android.util.SparseArray; import java.io.Serializable; import java.util.ArrayList; -<<<<<<< HEAD public class Bundle extends BaseBundle implements Cloneable, Parcelable { From 4f253590f184f05109d30e98c3e8b699d8f33d43 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 20 Jan 2022 14:10:02 +0100 Subject: [PATCH 09/13] Fix method name in LocalDatabaseOpenMethodAccess --- .../security/CleartextStorageAndroidDatabaseQuery.qll | 2 +- .../CWE-312/CleartextStorageAndroidDatabaseTest.java | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index e6273a18e8f6..0a1f306677f9 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -33,7 +33,7 @@ class LocalDatabaseOpenMethodAccess extends Storable, Call { m.hasName("getWritableDatabase") or m.getDeclaringType() instanceof TypeSQLiteDatabase and - m.hasName(["create", "open%Database", "compileStatement"]) + m.hasName(["create", "openDatabase", "openOrCreateDatabase", "compileStatement"]) or m.getDeclaringType().getASupertype*() instanceof TypeContext and m.hasName("openOrCreateDatabase") diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java index 0b4e317534c4..8dc61543ed65 100644 --- a/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java +++ b/java/ql/test/query-tests/security/CWE-312/CleartextStorageAndroidDatabaseTest.java @@ -19,14 +19,20 @@ public void testCleartextStorageAndroiDatabaseSafe2(Context ctx, String name, St db.execSQL("DROP TABLE passwords;"); // Safe - no sensitive value being stored } - public void testCleartextStorageAndroiDatabase1(Context ctx, String name, String password) { + public void testCleartextStorageAndroiDatabase0(Context ctx, String name, String password) { SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); String query = "INSERT INTO users VALUES ('" + name + "', '" + password + "');"; db.execSQL(query); // $ hasCleartextStorageAndroidDatabase } + public void testCleartextStorageAndroiDatabase1(Context ctx, String name, String password) { + SQLiteDatabase db = SQLiteDatabase.openDatabase("", null, 0); + String query = "INSERT INTO users VALUES ('" + name + "', '" + password + "');"; + db.execSQL(query); // $ hasCleartextStorageAndroidDatabase + } + public void testCleartextStorageAndroiDatabase2(String name, String password) { - SQLiteDatabase db = SQLiteDatabase.create(null); + SQLiteDatabase db = SQLiteDatabase.openOrCreateDatabase("", null); String query = "INSERT INTO users VALUES (?, ?)"; db.execSQL(query, new String[] {name, password}); // $ hasCleartextStorageAndroidDatabase } From c6dd7ddf7a7554deada68213e9503fb1d960ae32 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 20 Jan 2022 14:46:28 +0100 Subject: [PATCH 10/13] Fix stub --- .../android/database/sqlite/SQLiteDatabase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java index f6dd58334821..64b62e68d443 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/database/sqlite/SQLiteDatabase.java @@ -96,7 +96,6 @@ public void endTransaction(){} public void execPerConnectionSQL(String p0, Object[] p1){} public void execSQL(String p0){} public void execSQL(String p0, Object[] p1){} - public void execPerConnectionSQL (String p0, Object[] p1){} public void markTableSyncable(String p0, String p1){} public void markTableSyncable(String p0, String p1, String p2){} public void setCustomAggregateFunction(String p0, BinaryOperator p1){} From 4df0f399cd36448ab974e5015d02068652db1cc8 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 20 Jan 2022 14:49:57 +0100 Subject: [PATCH 11/13] Move ContentProvider models to the appropriate file --- .../code/java/dataflow/ExternalFlow.qll | 4 +-- .../code/java/frameworks/android/Android.qll | 36 ------------------- .../frameworks/android/ContentProviders.qll | 36 +++++++++++++++++++ 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index ab7404d10119..a95b27fca30c 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -78,10 +78,12 @@ private import FlowSummary private module Frameworks { private import internal.ContainerFlow private import semmle.code.java.frameworks.android.Android + private import semmle.code.java.frameworks.android.ContentProviders private import semmle.code.java.frameworks.android.Intent private import semmle.code.java.frameworks.android.Notifications private import semmle.code.java.frameworks.android.Slice private import semmle.code.java.frameworks.android.SQLite + private import semmle.code.java.frameworks.android.Widget private import semmle.code.java.frameworks.android.XssSinks private import semmle.code.java.frameworks.ApacheHttp private import semmle.code.java.frameworks.apache.Collections @@ -132,8 +134,6 @@ private module Frameworks { private import semmle.code.java.frameworks.Hibernate private import semmle.code.java.frameworks.jOOQ private import semmle.code.java.frameworks.spring.SpringHttp - private import semmle.code.java.frameworks.android.ContentProviders - private import semmle.code.java.frameworks.android.Widget } private predicate sourceModelCsv(string row) { diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Android.qll b/java/ql/lib/semmle/code/java/frameworks/android/Android.qll index 41ad2d14968c..b1a0d49e96f4 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Android.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Android.qll @@ -177,42 +177,6 @@ private class UriModel extends SummaryModelCsv { } } -private class ContentProviderSourceModels extends SourceModelCsv { - override predicate row(string row) { - row = - [ - // ContentInterface models are here for backwards compatibility (it was removed in API 28) - "android.content;ContentInterface;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider", - "android.content;ContentProvider;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider", - "android.content;ContentProvider;true;call;(String,String,Bundle);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;delete;(Uri,String,String[]);;Parameter[0..2];contentprovider", - "android.content;ContentInterface;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider", - "android.content;ContentProvider;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider", - "android.content;ContentInterface;true;getType;(Uri);;Parameter[0];contentprovider", - "android.content;ContentProvider;true;getType;(Uri);;Parameter[0];contentprovider", - "android.content;ContentInterface;true;insert;(Uri,ContentValues,Bundle);;Parameter[0];contentprovider", - "android.content;ContentProvider;true;insert;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;insert;(Uri,ContentValues);;Parameter[0..1];contentprovider", - "android.content;ContentInterface;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", - "android.content;ContentProvider;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", - "android.content;ContentProvider;true;openAssetFile;(Uri,String);;Parameter[0];contentprovider", - "android.content;ContentInterface;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle);;Parameter[0..2];contentprovider", - "android.content;ContentInterface;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", - "android.content;ContentProvider;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", - "android.content;ContentProvider;true;openFile;(Uri,String);;Parameter[0];contentprovider", - "android.content;ContentInterface;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;query;(Uri,String[],String,String[],String);;Parameter[0..4];contentprovider", - "android.content;ContentProvider;true;query;(Uri,String[],String,String[],String,CancellationSignal);;Parameter[0..4];contentprovider", - "android.content;ContentInterface;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", - "android.content;ContentProvider;true;update;(Uri,ContentValues,String,String[]);;Parameter[0..3];contentprovider" - ] - } -} - /** Interface for classes whose instances can be written to and restored from a Parcel. */ class TypeParcelable extends Interface { TypeParcelable() { this.hasQualifiedName("android.os", "Parcelable") } diff --git a/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll b/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll index abda2af3c035..42c22fcf8713 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.qll @@ -10,6 +10,42 @@ class ContentValues extends Class { ContentValues() { this.hasQualifiedName("android.content", "ContentValues") } } +private class ContentProviderSourceModels extends SourceModelCsv { + override predicate row(string row) { + row = + [ + // ContentInterface models are here for backwards compatibility (it was removed in API 28) + "android.content;ContentInterface;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider", + "android.content;ContentProvider;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider", + "android.content;ContentProvider;true;call;(String,String,Bundle);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;delete;(Uri,String,String[]);;Parameter[0..2];contentprovider", + "android.content;ContentInterface;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider", + "android.content;ContentProvider;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider", + "android.content;ContentInterface;true;getType;(Uri);;Parameter[0];contentprovider", + "android.content;ContentProvider;true;getType;(Uri);;Parameter[0];contentprovider", + "android.content;ContentInterface;true;insert;(Uri,ContentValues,Bundle);;Parameter[0];contentprovider", + "android.content;ContentProvider;true;insert;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;insert;(Uri,ContentValues);;Parameter[0..1];contentprovider", + "android.content;ContentInterface;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", + "android.content;ContentProvider;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", + "android.content;ContentProvider;true;openAssetFile;(Uri,String);;Parameter[0];contentprovider", + "android.content;ContentInterface;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle);;Parameter[0..2];contentprovider", + "android.content;ContentInterface;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", + "android.content;ContentProvider;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", + "android.content;ContentProvider;true;openFile;(Uri,String);;Parameter[0];contentprovider", + "android.content;ContentInterface;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;query;(Uri,String[],String,String[],String);;Parameter[0..4];contentprovider", + "android.content;ContentProvider;true;query;(Uri,String[],String,String[],String,CancellationSignal);;Parameter[0..4];contentprovider", + "android.content;ContentInterface;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", + "android.content;ContentProvider;true;update;(Uri,ContentValues,String,String[]);;Parameter[0..3];contentprovider" + ] + } +} + private class SummaryModels extends SummaryModelCsv { override predicate row(string row) { row = From 4f4f531dfcc39f7672e9bb441cdfc9fd85f01103 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 24 Jan 2022 15:13:09 +0100 Subject: [PATCH 12/13] Add missing QLDoc --- java/ql/lib/semmle/code/java/frameworks/Thrift.qll | 1 + .../lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll | 1 + 2 files changed, 2 insertions(+) diff --git a/java/ql/lib/semmle/code/java/frameworks/Thrift.qll b/java/ql/lib/semmle/code/java/frameworks/Thrift.qll index bf826cfe47ea..8d48ff3a4e95 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Thrift.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Thrift.qll @@ -25,6 +25,7 @@ class ThriftIface extends Interface { this.getFile() instanceof ThriftGeneratedFile } + /** Returns an implementation of a method of this interface. */ Method getAnImplementingMethod() { result.getDeclaringType().(Class).getASupertype+() = this and result.overrides+(this.getAMethod()) and diff --git a/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll b/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll index 5ee8f25724e0..5e176300c092 100644 --- a/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll +++ b/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll @@ -15,6 +15,7 @@ class StrutsAnnotation extends Annotation { class StrutsActionAnnotation extends StrutsAnnotation { StrutsActionAnnotation() { this.getType().hasName("Action") } + /** Returns a callable annotated with this annotation. */ Callable getActionCallable() { result = this.getAnnotatedElement() or From 54e8ea56e8e749a26863ff9ec58679617934699b Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 2 Feb 2022 15:44:26 +0100 Subject: [PATCH 13/13] Apply suggestions from code review Co-authored-by: Anders Schack-Mulligen --- java/ql/lib/semmle/code/java/frameworks/Thrift.qll | 2 +- java/ql/lib/semmle/code/java/frameworks/android/Widget.qll | 4 ++-- .../semmle/code/java/frameworks/struts/StrutsAnnotations.qll | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/java/ql/lib/semmle/code/java/frameworks/Thrift.qll b/java/ql/lib/semmle/code/java/frameworks/Thrift.qll index 8d48ff3a4e95..a441c6f7e56d 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Thrift.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Thrift.qll @@ -25,7 +25,7 @@ class ThriftIface extends Interface { this.getFile() instanceof ThriftGeneratedFile } - /** Returns an implementation of a method of this interface. */ + /** Gets an implementation of a method of this interface. */ Method getAnImplementingMethod() { result.getDeclaringType().(Class).getASupertype+() = this and result.overrides+(this.getAMethod()) and diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll b/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll index 961c63a4069c..c5eda29547bc 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Widget.qll @@ -6,7 +6,7 @@ import semmle.code.java.dataflow.FlowSources private class AndroidWidgetSourceModels extends SourceModelCsv { override predicate row(string row) { - row = ["android.widget;EditText;true;getText;;;ReturnValue;android-widget"] + row = "android.widget;EditText;true;getText;;;ReturnValue;android-widget" } } @@ -18,6 +18,6 @@ private class DefaultAndroidWidgetSources extends RemoteFlowSource { private class AndroidWidgetSummaryModels extends SummaryModelCsv { override predicate row(string row) { - row = ["android.widget;EditText;true;getText;;;Argument[-1];ReturnValue;taint"] + row = "android.widget;EditText;true;getText;;;Argument[-1];ReturnValue;taint" } } diff --git a/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll b/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll index 5e176300c092..27ada4d6ff03 100644 --- a/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll +++ b/java/ql/lib/semmle/code/java/frameworks/struts/StrutsAnnotations.qll @@ -15,7 +15,7 @@ class StrutsAnnotation extends Annotation { class StrutsActionAnnotation extends StrutsAnnotation { StrutsActionAnnotation() { this.getType().hasName("Action") } - /** Returns a callable annotated with this annotation. */ + /** Gets a callable annotated with this annotation. */ Callable getActionCallable() { result = this.getAnnotatedElement() or