diff options
author | Presubmit Automerger Backend <android-build-presubmit-automerger-backend@system.gserviceaccount.com> | 2022-04-25 22:35:11 +0000 |
---|---|---|
committer | Presubmit Automerger Backend <android-build-presubmit-automerger-backend@system.gserviceaccount.com> | 2022-04-25 22:35:11 +0000 |
commit | 56f84e89fcd2bf35693a976c69a365a10daca03c (patch) | |
tree | c0cedeb9b267b56bded10d0f80c8d00b2fc5a647 | |
parent | e49133bca0178ce4e9d0373051e97444862a9fef (diff) | |
parent | 74cf86e9b493220c5b3ecc0817eeb7e7188199c8 (diff) | |
download | ContactsProvider-56f84e89fcd2bf35693a976c69a365a10daca03c.tar.gz |
[automerge] DO NOT MERGE Add check that prevents file operations outside of Call Composer Dir 2p: 74cf86e9b4
Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/providers/ContactsProvider/+/17921289
Bug: 219015884
Change-Id: Ib764f3b511c19ddee64e46b98b82d5755fee63aa
3 files changed, 159 insertions, 6 deletions
diff --git a/src/com/android/providers/contacts/CallLogProvider.java b/src/com/android/providers/contacts/CallLogProvider.java index c832e9b4..991413eb 100644 --- a/src/com/android/providers/contacts/CallLogProvider.java +++ b/src/com/android/providers/contacts/CallLogProvider.java @@ -52,12 +52,15 @@ import android.telecom.TelecomManager; 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; @@ -651,6 +654,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); @@ -764,6 +768,8 @@ public class CallLogProvider extends ContentProvider { return null; } Path pathToFile = pathToCallComposerDir.resolve(fileName); + enforceValidCallLogPath(pathToCallComposerDir, pathToFile, + "allocateNewCallComposerPicture"); Files.createFile(pathToFile); if (forAllUsers) { @@ -777,10 +783,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; @@ -1045,8 +1051,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 = @@ -1221,4 +1228,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 92b4b171..885fbe04 100644 --- a/tests/src/com/android/providers/contacts/CallLogProviderTest.java +++ b/tests/src/com/android/providers/contacts/CallLogProviderTest.java @@ -18,16 +18,25 @@ package com.android.providers.contacts; 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 android.content.BroadcastReceiver; import android.content.ComponentName; import android.content.ContentProvider; import android.content.ContentUris; import android.content.ContentValues; +import android.content.Intent; import android.database.Cursor; +import android.database.DatabaseUtils; import android.database.MatrixCursor; +import android.database.sqlite.SQLiteDatabase; import android.net.Uri; import android.provider.CallLog; import android.provider.CallLog.Calls; @@ -35,10 +44,19 @@ import android.provider.ContactsContract; import android.provider.ContactsContract.CommonDataKinds.Phone; import android.provider.VoicemailContract.Voicemails; import android.telecom.PhoneAccountHandle; +import android.telecom.TelecomManager; +import android.telephony.SubscriptionInfo; import android.test.suitebuilder.annotation.MediumTest; +import org.junit.Assert; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; /** * Unit tests for {@link CallLogProvider}. @@ -66,6 +84,24 @@ public class CallLogProviderTest extends BaseContactsProvider2Test { private static final int MIN_MATCH = 7; + private static final long TEST_TIMEOUT = 5000; + + private static final String TELEPHONY_PACKAGE = "com.android.phone"; + private static final String TELEPHONY_CLASS + = "com.android.services.telephony.TelephonyConnectionService"; + private static final String TEST_PHONE_ACCOUNT_HANDLE_SUB_ID = "666"; + private static final int TEST_PHONE_ACCOUNT_HANDLE_SUB_ID_INT = 666; + private static final String TEST_PHONE_ACCOUNT_HANDLE_ICC_ID1 = "891004234814455936F"; + 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; @@ -281,6 +317,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"); } |