Skip to content

Commit

Permalink
Merge pull request from GHSA-vjp2-f63v-w479
Browse files Browse the repository at this point in the history
[stable-3.18] Harden FileContentProvider for public Uri paths
  • Loading branch information
AlvaroBrey committed Dec 22, 2021
2 parents 07b4ac3 + aa9edbc commit 627caba
Show file tree
Hide file tree
Showing 7 changed files with 593 additions and 53 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
*
* Nextcloud Android client application
*
* @author Tobias Kaminsky
* Copyright (C) 2021 Tobias Kaminsky
* Copyright (C) 2021 Nextcloud GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package com.owncloud.android.providers

import java.lang.IllegalArgumentException
import com.owncloud.android.db.ProviderMeta
import android.content.ContentValues
import com.owncloud.android.utils.MimeTypeUtil
import org.junit.Test

@Suppress("FunctionNaming")
class FileContentProviderVerificationIT {

companion object {
private const val INVALID_COLUMN = "Invalid column"
private const val FILE_LENGTH = 120
}

@Test(expected = IllegalArgumentException::class)
fun verifyColumnName_Exception() {
FileContentProvider.VerificationUtils.verifyColumnName(INVALID_COLUMN)
}

@Test
fun verifyColumnName_OK() {
FileContentProvider.VerificationUtils.verifyColumnName(ProviderMeta.ProviderTableMeta.FILE_NAME)
}

@Test
fun verifyColumn_ContentValues_OK() {
// with valid columns
val contentValues = ContentValues()
contentValues.put(ProviderMeta.ProviderTableMeta.FILE_CONTENT_LENGTH, FILE_LENGTH)
contentValues.put(ProviderMeta.ProviderTableMeta.FILE_CONTENT_TYPE, MimeTypeUtil.MIMETYPE_TEXT_MARKDOWN)
FileContentProvider.VerificationUtils.verifyColumns(contentValues)

// empty
FileContentProvider.VerificationUtils.verifyColumns(ContentValues())
}

@Test(expected = IllegalArgumentException::class)
fun verifyColumn_ContentValues_invalidColumn() {
// with invalid columns
val contentValues = ContentValues()
contentValues.put(INVALID_COLUMN, FILE_LENGTH)
contentValues.put(ProviderMeta.ProviderTableMeta.FILE_CONTENT_TYPE, MimeTypeUtil.MIMETYPE_TEXT_MARKDOWN)
FileContentProvider.VerificationUtils.verifyColumns(contentValues)
}

@Test
fun verifySortOrder_OK() {
// null
FileContentProvider.VerificationUtils.verifySortOrder(null)

// empty
FileContentProvider.VerificationUtils.verifySortOrder("")

// valid sort
FileContentProvider.VerificationUtils.verifySortOrder(ProviderMeta.ProviderTableMeta.FILE_DEFAULT_SORT_ORDER)
}

@Test(expected = IllegalArgumentException::class)
fun verifySortOrder_InvalidColumn() {
// with invalid column
FileContentProvider.VerificationUtils.verifySortOrder("$INVALID_COLUMN desc")
}

@Test(expected = IllegalArgumentException::class)
fun verifySortOrder_InvalidGrammar() {
// with invalid grammar
FileContentProvider.VerificationUtils.verifySortOrder("${ProviderMeta.ProviderTableMeta._ID} ;--foo")
}

@Test
fun verifyWhere_OK() {
FileContentProvider.VerificationUtils.verifyWhere(null)
FileContentProvider.VerificationUtils.verifyWhere(
"${ProviderMeta.ProviderTableMeta._ID}=? AND ${ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER}=?"
)
FileContentProvider.VerificationUtils.verifyWhere(
"${ProviderMeta.ProviderTableMeta._ID} = 1" +
" AND (1 = 1)" +
" AND ${ProviderMeta.ProviderTableMeta.FILE_ACCOUNT_OWNER} LIKE ?"
)
}

@Test(expected = IllegalArgumentException::class)
fun verifyWhere_InvalidColumnName() {
FileContentProvider.VerificationUtils.verifyWhere("$INVALID_COLUMN= ?")
}

@Test(expected = IllegalArgumentException::class)
fun verifyWhere_InvalidGrammar() {
FileContentProvider.VerificationUtils.verifyWhere("1=1 -- SELECT * FROM")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ public long storeExternalLink(ExternalLink externalLink) {
* @return numbers of rows deleted
*/
public int deleteAllExternalLinks() {
return mContentResolver.delete(ProviderMeta.ProviderTableMeta.CONTENT_URI_EXTERNAL_LINKS, " 1 = 1 ", null);
return mContentResolver.delete(ProviderMeta.ProviderTableMeta.CONTENT_URI_EXTERNAL_LINKS,
null,
null);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,10 @@ public boolean removeFile(OCFile ocFile, boolean removeDBData, boolean removeLoc
// ""+file.getFileId());
Uri file_uri = ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, ocFile.getFileId());
String where = ProviderTableMeta.FILE_ACCOUNT_OWNER + AND + ProviderTableMeta.FILE_PATH + "=?";

// TODO switch to string array with fixed keys in it
// file_owner = ? and path =?
// String[] { "file_owner", "path"}
String[] whereArgs = new String[]{account.name, ocFile.getRemotePath()};
int deleted = 0;
if (getContentProviderClient() != null) {
Expand Down Expand Up @@ -911,7 +915,7 @@ private List<OCFile> getFolderContent(long parentId, boolean onlyOnDevice) {
ProviderTableMeta.FILE_PARENT + "=?",
new String[]{String.valueOf(parentId)},
null
);
);
}

if (cursor != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ public int countEnabledSyncedFolders() {
*/
public List<SyncedFolder> getSyncedFolders() {
Cursor cursor = mContentResolver.query(
ProviderMeta.ProviderTableMeta.CONTENT_URI_SYNCED_FOLDERS,
null,
"1=1",
null,
null
);
ProviderMeta.ProviderTableMeta.CONTENT_URI_SYNCED_FOLDERS,
null,
null,
null,
null
);

if (cursor != null) {
List<SyncedFolder> list = new ArrayList<>(cursor.getCount());
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/owncloud/android/db/ProviderMeta.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,19 @@ static public class ProviderTableMeta implements BaseColumns {
_ID,
FILE_PARENT,
FILE_NAME,
FILE_ENCRYPTED_NAME,
FILE_CREATION,
FILE_MODIFIED,
FILE_MODIFIED_AT_LAST_SYNC_FOR_DATA,
FILE_CONTENT_LENGTH,
FILE_CONTENT_TYPE,
FILE_STORAGE_PATH,
FILE_PATH,
FILE_PATH_DECRYPTED,
FILE_ACCOUNT_OWNER,
FILE_LAST_SYNC_DATE,
FILE_LAST_SYNC_DATE_FOR_DATA,
FILE_KEEP_IN_SYNC,
FILE_ETAG,
FILE_ETAG_ON_SERVER,
FILE_SHARED_VIA_LINK,
Expand All @@ -146,6 +149,9 @@ static public class ProviderTableMeta implements BaseColumns {
FILE_MOUNT_TYPE,
FILE_HAS_PREVIEW,
FILE_UNREAD_COMMENTS_COUNT,
FILE_OWNER_ID,
FILE_OWNER_DISPLAY_NAME,
FILE_NOTE,
FILE_SHAREES,
FILE_RICH_WORKSPACE));

Expand Down
Loading

0 comments on commit 627caba

Please sign in to comment.