aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Stuart <tjstuart@google.com>2022-04-13 13:59:08 -0700
committerThomas Stuart <tjstuart@google.com>2022-04-26 00:15:27 +0000
commit5fd7ebb59d6c2b64255a4aad484d5f2217d4f4af (patch)
tree02028a047b8cc87cc1375977d6f76af38dbc1c43
parent7dbb1416a371bdbc9d2bbf85bef1a9b5d8288441 (diff)
downloadContactsProvider-5fd7ebb59d6c2b64255a4aad484d5f2217d4f4af.tar.gz
DO NOT MERGE 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
-rw-r--r--src/com/android/providers/contacts/CallLogProvider.java34
-rw-r--r--src/com/android/providers/contacts/util/FileUtilities.java50
-rw-r--r--tests/src/com/android/providers/contacts/CallLogProviderTest.java74
3 files changed, 152 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..b45ccaf7 100644
--- a/tests/src/com/android/providers/contacts/CallLogProviderTest.java
+++ b/tests/src/com/android/providers/contacts/CallLogProviderTest.java
@@ -18,9 +18,14 @@ 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.ComponentName;
import android.content.ContentProvider;
@@ -37,6 +42,11 @@ import android.provider.VoicemailContract.Voicemails;
import android.telecom.PhoneAccountHandle;
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;
@@ -66,6 +76,25 @@ 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 +310,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");
}