diff options
author | Thomas Stuart <tjstuart@google.com> | 2022-04-13 13:59:08 -0700 |
---|---|---|
committer | Thomas Stuart <tjstuart@google.com> | 2022-04-20 16:12:00 -0700 |
commit | c5da1394740292b036fa0d0b7ad9b96f0851b799 (patch) | |
tree | 7096aadbd360fab652740e922b1fa42847810ed1 | |
parent | 809e04f27d64ceb928ec7ad154be7f66937307d2 (diff) | |
download | ContactsProvider-c5da1394740292b036fa0d0b7ad9b96f0851b799.tar.gz |
Add check that prevents file operations outside of Call Composer Dir
Cannot open, delete, sync, or insert files outside of the Call Composer
directory. New check prevents this.
bug: 219015884
Test: 3 UT,
1. CallLogProviderTest#testOpenFileOutsideOfScopeThrowsException
2. CallLogProviderTest#testDeleteFileOutsideOfScopeThrowsException
3. CallLogProviderTest#testInsertFileOutsideOfScopeThrowsException
Change-Id: I5a9dc98db446707373479fe1c2cb5fba44bdedf7
3 files changed, 135 insertions, 7 deletions
diff --git a/src/com/android/providers/contacts/CallLogProvider.java b/src/com/android/providers/contacts/CallLogProvider.java index 948adaa4..0722d691 100644 --- a/src/com/android/providers/contacts/CallLogProvider.java +++ b/src/com/android/providers/contacts/CallLogProvider.java @@ -57,12 +57,14 @@ import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; import android.text.TextUtils; import android.util.ArrayMap; +import android.util.EventLog; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ProviderAccessStats; import com.android.providers.contacts.CallLogDatabaseHelper.DbProperties; import com.android.providers.contacts.CallLogDatabaseHelper.Tables; +import com.android.providers.contacts.util.FileUtilities; import com.android.providers.contacts.util.NeededForTesting; import com.android.providers.contacts.util.SelectionBuilder; import com.android.providers.contacts.util.UserUtils; @@ -723,6 +725,7 @@ public class CallLogProvider extends ContentProvider { throw new FileNotFoundException(uri.toString() + " does not correspond to a valid file."); } + enforceValidCallLogPath(callComposerDir, pictureFile,"openFile"); return ParcelFileDescriptor.open(pictureFile.toFile(), modeInt); } catch (IOException e) { Log.e(TAG, "IOException while opening call composer file: " + e); @@ -836,6 +839,8 @@ public class CallLogProvider extends ContentProvider { return null; } Path pathToFile = pathToCallComposerDir.resolve(fileName); + enforceValidCallLogPath(pathToCallComposerDir, pathToFile, + "allocateNewCallComposerPicture"); Files.createFile(pathToFile); if (forAllUsers) { @@ -849,10 +854,10 @@ public class CallLogProvider extends ContentProvider { private int deleteCallComposerPicture(Uri uri) { try { Path pathToCallComposerDir = getCallComposerPictureDirectory(getContext(), uri); - String fileName = uri.getLastPathSegment(); - boolean successfulDelete = - Files.deleteIfExists(pathToCallComposerDir.resolve(fileName)); - return successfulDelete ? 1 : 0; + Path fileToDelete = pathToCallComposerDir.resolve(uri.getLastPathSegment()); + enforceValidCallLogPath(pathToCallComposerDir, fileToDelete, + "deleteCallComposerPicture"); + return Files.deleteIfExists(fileToDelete) ? 1 : 0; } catch (IOException e) { Log.e(TAG, "IOException encountered deleting the call composer pics dir " + e); return 0; @@ -1111,8 +1116,9 @@ public class CallLogProvider extends ContentProvider { for (Uri uri : urisToCopy) { try { Uri uriWithUser = ContentProvider.maybeAddUserId(uri, sourceUserId); - Path newFilePath = getCallComposerPictureDirectory(getContext(), false) - .resolve(uri.getLastPathSegment()); + Path callComposerDir = getCallComposerPictureDirectory(getContext(), false); + Path newFilePath = callComposerDir.resolve(uri.getLastPathSegment()); + enforceValidCallLogPath(callComposerDir, newFilePath,"syncCallComposerPics"); try (ParcelFileDescriptor remoteFile = contentResolver.openFile(uriWithUser, "r", null); OutputStream localOut = @@ -1303,4 +1309,19 @@ public class CallLogProvider extends ContentProvider { public void dump(FileDescriptor fd, PrintWriter writer, String[] args) { mStats.dump(writer, " "); } + + /** + * Enforces a stricter check on what files the CallLogProvider can perform file operations on. + * @param rootPath where all valid new/existing paths should pass through. + * @param pathToCheck newly created path that is requesting a file op. (open, delete, etc.) + * @param callingMethod the calling method. Used only for debugging purposes. + */ + private void enforceValidCallLogPath(Path rootPath, Path pathToCheck, String callingMethod){ + if (!FileUtilities.isSameOrSubDirectory(rootPath.toFile(), pathToCheck.toFile())) { + EventLog.writeEvent(0x534e4554, "219015884", Binder.getCallingUid(), + (callingMethod + ": invalid uri passed")); + throw new SecurityException( + FileUtilities.INVALID_CALL_LOG_PATH_EXCEPTION_MESSAGE + pathToCheck); + } + } } diff --git a/src/com/android/providers/contacts/util/FileUtilities.java b/src/com/android/providers/contacts/util/FileUtilities.java new file mode 100644 index 00000000..4772423c --- /dev/null +++ b/src/com/android/providers/contacts/util/FileUtilities.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2022 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 com.android.providers.contacts.util; + +import android.util.Log; + +import java.io.File; +import java.io.IOException; + +public final class FileUtilities { + + public static final String TAG = FileUtilities.class.getSimpleName(); + public static final String INVALID_CALL_LOG_PATH_EXCEPTION_MESSAGE = + "Invalid [Call Log] path. Cannot operate on file:"; + + /** + * Checks, whether the child directory is the same as, or a sub-directory of the base + * directory. + */ + public static boolean isSameOrSubDirectory(File base, File child) { + try { + File basePath = base.getCanonicalFile(); + File currPath = child.getCanonicalFile(); + while (currPath != null) { + if (basePath.equals(currPath)) { + return true; + } + currPath = currPath.getParentFile(); // pops sub-dir + } + return false; + } catch (IOException ex) { + Log.e(TAG, "Error while accessing file", ex); + return false; + } + } +} diff --git a/tests/src/com/android/providers/contacts/CallLogProviderTest.java b/tests/src/com/android/providers/contacts/CallLogProviderTest.java index ebb06556..c7555823 100644 --- a/tests/src/com/android/providers/contacts/CallLogProviderTest.java +++ b/tests/src/com/android/providers/contacts/CallLogProviderTest.java @@ -21,9 +21,11 @@ import static android.provider.CallLog.Calls.MISSED_REASON_NOT_MISSED; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.when; +import android.content.ContentResolver; import android.telecom.CallerInfo; import com.android.providers.contacts.testutil.CommonDatabaseUtils; import com.android.providers.contacts.util.ContactsPermissions; +import com.android.providers.contacts.util.FileUtilities; import com.android.providers.contacts.util.PhoneAccountHandleMigrationUtils; import android.content.BroadcastReceiver; @@ -46,8 +48,10 @@ import android.telecom.PhoneAccountHandle; import android.telecom.TelecomManager; import android.telephony.SubscriptionInfo; import android.test.suitebuilder.annotation.MediumTest; -import android.util.Log; +import org.junit.Assert; + +import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -92,6 +96,14 @@ public class CallLogProviderTest extends BaseContactsProvider2Test { private static final String TEST_PHONE_ACCOUNT_HANDLE_ICC_ID2 = "891004234814455937"; private static final String TEST_COMPONENT_NAME = "foo/bar"; + private static final Uri INVALID_CALL_LOG_URI = Uri.parse( + "content://call_log/call_composer/%2fdata%2fdata%2fcom.android.providers" + + ".contacts%2fshared_prefs%2fContactsUpgradeReceiver.xml"); + + private static final String TEST_FAIL_DID_NOT_TRHOW_SE = + "fail test because Security Exception was not throw"; + + private int mOldMinMatch; private CallLogProviderTestable mCallLogProvider; @@ -515,6 +527,51 @@ public class CallLogProviderTest extends BaseContactsProvider2Test { } } + /** + * Tests scenario where an app gives {@link ContentResolver} a file to open that is not in the + * Call Log Provider directory. + */ + public void testOpenFileOutsideOfScopeThrowsException() throws FileNotFoundException { + try { + mResolver.openFile(INVALID_CALL_LOG_URI, "w", null); + // previous line should throw exception + fail(TEST_FAIL_DID_NOT_TRHOW_SE); + } catch (SecurityException e) { + Assert.assertTrue( + e.toString().contains(FileUtilities.INVALID_CALL_LOG_PATH_EXCEPTION_MESSAGE)); + } + } + + /** + * Tests scenario where an app gives {@link ContentResolver} a file to delete that is not in the + * Call Log Provider directory. + */ + public void testDeleteFileOutsideOfScopeThrowsException() { + try { + mResolver.delete(INVALID_CALL_LOG_URI, "w", null); + // previous line should throw exception + fail(TEST_FAIL_DID_NOT_TRHOW_SE); + } catch (SecurityException e) { + Assert.assertTrue( + e.toString().contains(FileUtilities.INVALID_CALL_LOG_PATH_EXCEPTION_MESSAGE)); + } + } + + /** + * Tests scenario where an app gives {@link ContentResolver} a file to insert outside the + * Call Log Provider directory. + */ + public void testInsertFileOutsideOfScopeThrowsException() { + try { + mResolver.insert(INVALID_CALL_LOG_URI, new ContentValues()); + // previous line should throw exception + fail(TEST_FAIL_DID_NOT_TRHOW_SE); + } catch (SecurityException e) { + Assert.assertTrue( + e.toString().contains(FileUtilities.INVALID_CALL_LOG_PATH_EXCEPTION_MESSAGE)); + } + } + public void testUriWithBadLimitParamThrowsException() { assertParamThrowsIllegalArgumentException(Calls.LIMIT_PARAM_KEY, "notvalid"); } |