Skip to content

Commit ac35a97

Browse files
Jack Heandi34
authored andcommitted
OPP: Restrict file based URI access to external storage
* Allow only external storage paths in file based URI in BluetoothOppSendFileInfo when the file send request comes from an external source * Fix a potential NPE when using Uri.getPath() Bug: 35310991 Test: Make, test various cases of Bluetooth file share Change-Id: Id5519588266f22674b6d01ec6ebe34fe38cbccce (cherry picked from commit 3edd7f0a8aadf2f44bc62ea5b567c74d39a534c8) (cherry picked from commit 3ce229fa602f739d1e98e2a85a3bd955c3a308f6)
1 parent 812499f commit ac35a97

File tree

6 files changed

+66
-12
lines changed

6 files changed

+66
-12
lines changed

src/com/android/bluetooth/opp/BluetoothOppHandoverReceiver.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void onReceive(Context context, Intent intent) {
5050
// Save type/stream, will be used when adding transfer
5151
// session to DB.
5252
BluetoothOppManager.getInstance(context).saveSendingFileInfo(type,
53-
stream.toString(), true);
53+
stream.toString(), true, false);
5454
} else {
5555
if (D) Log.d(TAG, "No mimeType or stream attached to handover request");
5656
}
@@ -60,7 +60,7 @@ public void onReceive(Context context, Intent intent) {
6060
uris = intent.getParcelableArrayListExtra(Intent.EXTRA_STREAM);
6161
if (mimeType != null && uris != null) {
6262
BluetoothOppManager.getInstance(context).saveSendingFileInfo(mimeType,
63-
uris, true);
63+
uris, true /* isHandover */, true /* fromExternal */);
6464
} else {
6565
if (D) Log.d(TAG, "No mimeType or stream attached to handover request");
6666
return;

src/com/android/bluetooth/opp/BluetoothOppLauncherActivity.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ public void onCreate(Bundle savedInstanceState) {
103103
Thread t = new Thread(new Runnable() {
104104
public void run() {
105105
BluetoothOppManager.getInstance(BluetoothOppLauncherActivity.this)
106-
.saveSendingFileInfo(type,stream.toString(), false);
106+
.saveSendingFileInfo(type,stream.toString(),
107+
false /* isHandover */, true /* fromExternal */);
107108
//Done getting file info..Launch device picker and finish this activity
108109
launchDevicePicker();
109110
finish();
@@ -119,7 +120,8 @@ public void run() {
119120
Thread t = new Thread(new Runnable() {
120121
public void run() {
121122
BluetoothOppManager.getInstance(BluetoothOppLauncherActivity.this)
122-
.saveSendingFileInfo(type,fileUri.toString(), false);
123+
.saveSendingFileInfo(type,fileUri.toString(),
124+
false /* isHandover */, false /* fromExternal */);
123125
//Done getting file info..Launch device picker
124126
//and finish this activity
125127
launchDevicePicker();
@@ -147,7 +149,8 @@ public void run() {
147149
Thread t = new Thread(new Runnable() {
148150
public void run() {
149151
BluetoothOppManager.getInstance(BluetoothOppLauncherActivity.this)
150-
.saveSendingFileInfo(mimeType,uris, false);
152+
.saveSendingFileInfo(mimeType,uris,
153+
false /* isHandover */, true /* fromExternal */);
151154
//Done getting file info..Launch device picker
152155
//and finish this activity
153156
launchDevicePicker();

src/com/android/bluetooth/opp/BluetoothOppManager.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,30 +247,32 @@ private void storeApplicationData() {
247247
if (V) Log.v(TAG, "Application data stored to SharedPreference! ");
248248
}
249249

250-
public void saveSendingFileInfo(String mimeType, String uriString, boolean isHandover) {
250+
public void saveSendingFileInfo(String mimeType, String uriString, boolean isHandover,
251+
boolean fromExternal) {
251252
synchronized (BluetoothOppManager.this) {
252253
mMultipleFlag = false;
253254
mMimeTypeOfSendingFile = mimeType;
254255
mIsHandoverInitiated = isHandover;
255256
Uri uri = Uri.parse(uriString);
256257
BluetoothOppSendFileInfo sendFileInfo =
257-
BluetoothOppSendFileInfo.generateFileInfo(mContext, uri, mimeType);
258+
BluetoothOppSendFileInfo.generateFileInfo(mContext, uri, mimeType, fromExternal);
258259
uri = BluetoothOppUtility.generateUri(uri, sendFileInfo);
259260
BluetoothOppUtility.putSendFileInfo(uri, sendFileInfo);
260261
mUriOfSendingFile = uri.toString();
261262
storeApplicationData();
262263
}
263264
}
264265

265-
public void saveSendingFileInfo(String mimeType, ArrayList<Uri> uris, boolean isHandover) {
266+
public void saveSendingFileInfo(String mimeType, ArrayList<Uri> uris, boolean isHandover,
267+
boolean fromExternal) {
266268
synchronized (BluetoothOppManager.this) {
267269
mMultipleFlag = true;
268270
mMimeTypeOfSendingFiles = mimeType;
269271
mUrisOfSendingFiles = new ArrayList<Uri>();
270272
mIsHandoverInitiated = isHandover;
271273
for (Uri uri : uris) {
272274
BluetoothOppSendFileInfo sendFileInfo =
273-
BluetoothOppSendFileInfo.generateFileInfo(mContext, uri, mimeType);
275+
BluetoothOppSendFileInfo.generateFileInfo(mContext, uri, mimeType, fromExternal);
274276
uri = BluetoothOppUtility.generateUri(uri, sendFileInfo);
275277
mUrisOfSendingFiles.add(uri);
276278
BluetoothOppUtility.putSendFileInfo(uri, sendFileInfo);

src/com/android/bluetooth/opp/BluetoothOppSendFileInfo.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import android.database.sqlite.SQLiteException;
4040
import android.net.Uri;
4141
import android.provider.OpenableColumns;
42+
import android.util.EventLog;
4243
import android.util.Log;
4344

4445
import java.io.File;
@@ -97,8 +98,8 @@ public BluetoothOppSendFileInfo(String data, String type, long length, int statu
9798
mStatus = status;
9899
}
99100

100-
public static BluetoothOppSendFileInfo generateFileInfo(Context context, Uri uri,
101-
String type) {
101+
public static BluetoothOppSendFileInfo generateFileInfo(
102+
Context context, Uri uri, String type, boolean fromExternal) {
102103
ContentResolver contentResolver = context.getContentResolver();
103104
String scheme = uri.getScheme();
104105
String fileName = null;
@@ -134,6 +135,16 @@ public static BluetoothOppSendFileInfo generateFileInfo(Context context, Uri uri
134135
fileName = uri.getLastPathSegment();
135136
}
136137
} else if ("file".equals(scheme)) {
138+
if (uri.getPath() == null) {
139+
Log.e(TAG, "Invalid URI path: " + uri);
140+
return SEND_FILE_INFO_ERROR;
141+
}
142+
if (fromExternal && !BluetoothOppUtility.isInExternalStorageDir(uri)) {
143+
EventLog.writeEvent(0x534e4554, "35310991", -1, uri.getPath());
144+
Log.e(TAG,
145+
"File based URI not in Environment.getExternalStorageDirectory() is not allowed.");
146+
return SEND_FILE_INFO_ERROR;
147+
}
137148
fileName = uri.getLastPathSegment();
138149
contentType = type;
139150
File f = new File(uri.getPath());

src/com/android/bluetooth/opp/BluetoothOppTransferActivity.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ public void onClick(DialogInterface dialog, int which) {
373373
// retry the failed transfer
374374
Uri uri = BluetoothOppUtility.originalUri(Uri.parse(mTransInfo.mFileUri));
375375
BluetoothOppSendFileInfo sendFileInfo =
376-
BluetoothOppSendFileInfo.generateFileInfo(this, uri, mTransInfo.mFileType);
376+
BluetoothOppSendFileInfo.generateFileInfo(this, uri, mTransInfo.mFileType, false);
377377
uri = BluetoothOppUtility.generateUri(uri, sendFileInfo);
378378
BluetoothOppUtility.putSendFileInfo(uri, sendFileInfo);
379379
mTransInfo.mFileUri = uri.toString();

src/com/android/bluetooth/opp/BluetoothOppUtility.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@
3939
import android.bluetooth.BluetoothAdapter;
4040
import android.bluetooth.BluetoothDevice;
4141
import android.net.Uri;
42+
import android.content.ContentResolver;
4243
import android.content.ContentValues;
4344
import android.content.Context;
4445
import android.content.ActivityNotFoundException;
4546
import android.content.Intent;
4647
import android.content.pm.PackageManager;
4748
import android.content.pm.ResolveInfo;
4849
import android.database.Cursor;
50+
import android.os.Environment;
4951
import android.util.Log;
5052

5153
import java.io.File;
@@ -349,4 +351,40 @@ static void closeSendFileInfo(Uri uri) {
349351
}
350352
}
351353
}
354+
355+
/**
356+
* Checks if the URI is in Environment.getExternalStorageDirectory() as it
357+
* is the only directory that is possibly readable by both the sender and
358+
* the Bluetooth process.
359+
*/
360+
static boolean isInExternalStorageDir(Uri uri) {
361+
if (!ContentResolver.SCHEME_FILE.equals(uri.getScheme())) {
362+
Log.e(TAG, "Not a file URI: " + uri);
363+
return false;
364+
}
365+
final File file = new File(uri.getCanonicalUri().getPath());
366+
return isSameOrSubDirectory(Environment.getExternalStorageDirectory(), file);
367+
}
368+
369+
/**
370+
* Checks, whether the child directory is the same as, or a sub-directory of the base
371+
* directory. Neither base nor child should be null.
372+
*/
373+
static boolean isSameOrSubDirectory(File base, File child) {
374+
try {
375+
base = base.getCanonicalFile();
376+
child = child.getCanonicalFile();
377+
File parentFile = child;
378+
while (parentFile != null) {
379+
if (base.equals(parentFile)) {
380+
return true;
381+
}
382+
parentFile = parentFile.getParentFile();
383+
}
384+
return false;
385+
} catch (IOException ex) {
386+
Log.e(TAG, "Error while accessing file", ex);
387+
return false;
388+
}
389+
}
352390
}

0 commit comments

Comments
 (0)