Skip to content

Commit

Permalink
Fixed leak of cross user data in multiple settings.
Browse files Browse the repository at this point in the history
  - Any app is allowed to receive GET_CONTENT intent. Using this, an user puts back in the intent an uri with data of another user.
  - Telephony service has INTERACT_ACROSS_USER permission. Using this, it reads and shows the deta to the evil user.

Fix: When telephony service gets the intent result, it checks if the uri is from the current user or not.

Bug: b/256591023 , b/256819787

Test: The malicious behaviour was not being reproduced. Unable to import contact from other users data.
Test2: Able to import contact from the primary user or uri with no user id
(These settings are not available for secondary users)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ab593467e900d4a6d25a34024a06195ae863f6dc)
Merged-In: I1e3a643f17948153aecc1d0df9ffd9619ad678c1
Change-Id: I1e3a643f17948153aecc1d0df9ffd9619ad678c1
  • Loading branch information
Ashish Kumar authored and thestinger committed Sep 6, 2023
1 parent 7f2817f commit b96ee4a
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/com/android/phone/CdmaCallForwardOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@
package com.android.phone;

import android.app.ActionBar;
import android.content.ContentProvider;
import android.content.Intent;
import android.database.Cursor;
import android.os.Bundle;
import android.os.PersistableBundle;
import android.os.Process;
import android.os.UserHandle;
import android.preference.Preference;
import android.preference.PreferenceScreen;
import android.telephony.CarrierConfigManager;
Expand Down Expand Up @@ -212,6 +215,15 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
}
Cursor cursor = null;
try {
// check if the URI returned by the user belongs to the user
final int currentUser = UserHandle.getUserId(Process.myUid());
if (currentUser
!= ContentProvider.getUserIdFromUri(data.getData(), currentUser)) {

Log.w(LOG_TAG, "onActivityResult: Contact data of different user, "
+ "cannot access");
return;
}
cursor = getContentResolver().query(data.getData(),
NUM_PROJECTION, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {
Expand Down
12 changes: 12 additions & 0 deletions src/com/android/phone/GsmUmtsCallForwardOptions.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.android.phone;

import android.app.ActionBar;
import android.content.ContentProvider;
import android.content.Intent;
import android.database.Cursor;
import android.os.Bundle;
import android.os.PersistableBundle;
import android.os.Process;
import android.os.UserHandle;
import android.preference.Preference;
import android.preference.PreferenceScreen;
import android.telephony.CarrierConfigManager;
Expand Down Expand Up @@ -203,6 +206,15 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
}
Cursor cursor = null;
try {
// check if the URI returned by the user belongs to the user
final int currentUser = UserHandle.getUserId(Process.myUid());
if (currentUser
!= ContentProvider.getUserIdFromUri(data.getData(), currentUser)) {

Log.w(LOG_TAG, "onActivityResult: Contact data of different user, "
+ "cannot access");
return;
}
cursor = getContentResolver().query(data.getData(),
NUM_PROJECTION, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {
Expand Down
14 changes: 14 additions & 0 deletions src/com/android/phone/settings/VoicemailSettingsActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.android.phone.settings;

import android.app.Dialog;
import android.content.ContentProvider;
import android.content.DialogInterface;
import android.content.Intent;
import android.database.Cursor;
Expand All @@ -25,6 +26,8 @@
import android.os.Handler;
import android.os.Message;
import android.os.PersistableBundle;
import android.os.Process;
import android.os.UserHandle;
import android.os.UserManager;
import android.preference.Preference;
import android.preference.PreferenceActivity;
Expand Down Expand Up @@ -520,6 +523,17 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {

Cursor cursor = null;
try {
// check if the URI returned by the user belongs to the user
final int currentUser = UserHandle.getUserId(Process.myUid());
if (currentUser
!= ContentProvider.getUserIdFromUri(data.getData(), currentUser)) {

if (DBG) {
log("onActivityResult: Contact data of different user, "
+ "cannot access");
}
return;
}
cursor = getContentResolver().query(data.getData(),
new String[] { CommonDataKinds.Phone.NUMBER }, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {
Expand Down
11 changes: 11 additions & 0 deletions src/com/android/phone/settings/fdn/EditFdnContactScreen.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@

import static android.app.Activity.RESULT_OK;

import android.content.ContentProvider;
import android.content.ContentValues;
import android.content.Intent;
import android.content.res.Resources;
import android.database.Cursor;
import android.net.Uri;
import android.os.Bundle;
import android.os.PersistableBundle;
import android.os.Process;
import android.os.UserHandle;
import android.provider.ContactsContract.CommonDataKinds;
import android.telephony.CarrierConfigManager;
import android.telephony.PhoneNumberUtils;
Expand Down Expand Up @@ -137,6 +140,14 @@ protected void onActivityResult(int requestCode, int resultCode, Intent intent)
}
Cursor cursor = null;
try {
// check if the URI returned by the user belongs to the user
final int currentUser = UserHandle.getUserId(Process.myUid());
if (currentUser
!= ContentProvider.getUserIdFromUri(intent.getData(), currentUser)) {
Log.w(LOG_TAG, "onActivityResult: Contact data of different user, "
+ "cannot access");
return;
}
cursor = getContentResolver().query(intent.getData(),
NUM_PROJECTION, null, null, null);
if ((cursor == null) || (!cursor.moveToFirst())) {
Expand Down

0 comments on commit b96ee4a

Please sign in to comment.