Skip to content

Commit a2dea4f

Browse files
jsharkeyAndroid (Google) Code Review
authored andcommitted
Merge "Better exceptions around provider permissions."
2 parents 80a6b33 + e5d4933 commit a2dea4f

File tree

1 file changed

+41
-46
lines changed

1 file changed

+41
-46
lines changed

core/java/android/content/ContentProvider.java

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -270,19 +270,24 @@ public ICancellationSignal createCancellationSignal() throws RemoteException {
270270
return CancellationSignal.createTransport();
271271
}
272272

273-
private boolean hasReadPermission(Uri uri) {
273+
private void enforceReadPermission(Uri uri) throws SecurityException {
274274
final Context context = getContext();
275275
final int pid = Binder.getCallingPid();
276276
final int uid = Binder.getCallingUid();
277+
String missingPerm = null;
277278

278279
if (uid == mMyUid) {
279-
return true;
280+
return;
281+
}
280282

281-
} else if (mExported) {
283+
if (mExported) {
282284
final String componentPerm = getReadPermission();
283-
if (componentPerm != null
284-
&& (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED)) {
285-
return true;
285+
if (componentPerm != null) {
286+
if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) {
287+
return;
288+
} else {
289+
missingPerm = componentPerm;
290+
}
286291
}
287292

288293
// track if unprotected read is allowed; any denied
@@ -296,56 +301,54 @@ private boolean hasReadPermission(Uri uri) {
296301
final String pathPerm = pp.getReadPermission();
297302
if (pathPerm != null && pp.match(path)) {
298303
if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) {
299-
return true;
304+
return;
300305
} else {
301306
// any denied <path-permission> means we lose
302307
// default <provider> access.
303308
allowDefaultRead = false;
309+
missingPerm = pathPerm;
304310
}
305311
}
306312
}
307313
}
308314

309315
// if we passed <path-permission> checks above, and no default
310316
// <provider> permission, then allow access.
311-
if (allowDefaultRead) return true;
317+
if (allowDefaultRead) return;
312318
}
313319

314320
// last chance, check against any uri grants
315321
if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION)
316322
== PERMISSION_GRANTED) {
317-
return true;
318-
}
319-
320-
return false;
321-
}
322-
323-
private void enforceReadPermission(Uri uri) {
324-
if (hasReadPermission(uri)) {
325323
return;
326324
}
327325

328-
String msg = "Permission Denial: reading "
329-
+ ContentProvider.this.getClass().getName()
330-
+ " uri " + uri + " from pid=" + Binder.getCallingPid()
331-
+ ", uid=" + Binder.getCallingUid()
332-
+ " requires " + getReadPermission();
333-
throw new SecurityException(msg);
326+
final String failReason = mExported
327+
? " requires " + missingPerm + ", or grantUriPermission()"
328+
: " requires the provider be exported, or grantUriPermission()";
329+
throw new SecurityException("Permission Denial: reading "
330+
+ ContentProvider.this.getClass().getName() + " uri " + uri + " from pid=" + pid
331+
+ ", uid=" + uid + failReason);
334332
}
335333

336-
private boolean hasWritePermission(Uri uri) {
334+
private void enforceWritePermission(Uri uri) throws SecurityException {
337335
final Context context = getContext();
338336
final int pid = Binder.getCallingPid();
339337
final int uid = Binder.getCallingUid();
338+
String missingPerm = null;
340339

341340
if (uid == mMyUid) {
342-
return true;
341+
return;
342+
}
343343

344-
} else if (mExported) {
344+
if (mExported) {
345345
final String componentPerm = getWritePermission();
346-
if (componentPerm != null
347-
&& (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED)) {
348-
return true;
346+
if (componentPerm != null) {
347+
if (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED) {
348+
return;
349+
} else {
350+
missingPerm = componentPerm;
351+
}
349352
}
350353

351354
// track if unprotected write is allowed; any denied
@@ -359,45 +362,37 @@ private boolean hasWritePermission(Uri uri) {
359362
final String pathPerm = pp.getWritePermission();
360363
if (pathPerm != null && pp.match(path)) {
361364
if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) {
362-
return true;
365+
return;
363366
} else {
364367
// any denied <path-permission> means we lose
365368
// default <provider> access.
366369
allowDefaultWrite = false;
370+
missingPerm = pathPerm;
367371
}
368372
}
369373
}
370374
}
371375

372376
// if we passed <path-permission> checks above, and no default
373377
// <provider> permission, then allow access.
374-
if (allowDefaultWrite) return true;
378+
if (allowDefaultWrite) return;
375379
}
376380

377381
// last chance, check against any uri grants
378382
if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_WRITE_URI_PERMISSION)
379383
== PERMISSION_GRANTED) {
380-
return true;
381-
}
382-
383-
return false;
384-
}
385-
386-
private void enforceWritePermission(Uri uri) {
387-
if (hasWritePermission(uri)) {
388384
return;
389385
}
390-
391-
String msg = "Permission Denial: writing "
392-
+ ContentProvider.this.getClass().getName()
393-
+ " uri " + uri + " from pid=" + Binder.getCallingPid()
394-
+ ", uid=" + Binder.getCallingUid()
395-
+ " requires " + getWritePermission();
396-
throw new SecurityException(msg);
386+
387+
final String failReason = mExported
388+
? " requires " + missingPerm + ", or grantUriPermission()"
389+
: " requires the provider be exported, or grantUriPermission()";
390+
throw new SecurityException("Permission Denial: writing "
391+
+ ContentProvider.this.getClass().getName() + " uri " + uri + " from pid=" + pid
392+
+ ", uid=" + uid + failReason);
397393
}
398394
}
399395

400-
401396
/**
402397
* Retrieves the Context this provider is running in. Only available once
403398
* {@link #onCreate} has been called -- this will return null in the

0 commit comments

Comments
 (0)