From 9bce52c0b988bda706da72b44c86cba2fef3935e Mon Sep 17 00:00:00 2001
From: Zim <zezeozue@google.com>
Date: Mon, 12 Jul 2021 15:31:22 +0100
Subject: [PATCH] Fix wrong LEGACY_STORAGE appop grant

Apps with WRITE_MEDIA_STORAGE permission should only be granted the
LEGACY_STORAGE appop while they target <R. This was the original
intention, but Ic24372348118ad9ed818a28f377e0decc78b9ecc changed
the behavior to allow the legacy appop for apps with
WRITE_MEDIA_STORAGE permission regardless of target SDK.

This change prevents granting the legacy storage appop to
apps with WRITE_MEDIA_STORAGE permission if they target
>=R. Additionally, we also deny the appop if they target >=R.

Bug: 190001005
Test: atest RestrictedStoragePermissionTest
Test: atest PreserveLegacyStorageHostTest
Test: Manually installed a targetR app with WRITE_MEDIA_STORAGE
permission and verified that legacy_storage was ignored
Change-Id: Icd804e289491a2416bcb9b5f78012314027af047
---
 .../SoftRestrictedPermissionPolicy.java       | 22 ++++++++++---------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java b/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java
index 9026262db8971..ab71355267468 100644
--- a/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java
+++ b/services/core/java/com/android/server/policy/SoftRestrictedPermissionPolicy.java
@@ -189,12 +189,16 @@ public abstract class SoftRestrictedPermissionPolicy {
                             return false;
                         }
 
-                        // 3. The app has WRITE_MEDIA_STORAGE, OR
-                        //      the app already has legacy external storage or requested it,
-                        //      and is < R.
-                        return hasWriteMediaStorageGrantedForUid
-                                || ((hasLegacyExternalStorage || hasRequestedLegacyExternalStorage)
-                                    && targetSDK < Build.VERSION_CODES.R);
+                        // 3. The app targetSDK should be less than R
+                        if (targetSDK >= Build.VERSION_CODES.R) {
+                            return false;
+                        }
+
+                        // 4. The app has WRITE_MEDIA_STORAGE,
+                        //    OR the app already has legacy external storage
+                        //    OR the app requested legacy external storage
+                        return hasWriteMediaStorageGrantedForUid || hasLegacyExternalStorage
+                                || hasRequestedLegacyExternalStorage;
                     }
                     @Override
                     public boolean mayDenyExtraAppOpIfGranted() {
@@ -216,10 +220,8 @@ public abstract class SoftRestrictedPermissionPolicy {
                             return true;
                         }
 
-                        // The package doesn't have WRITE_MEDIA_STORAGE,
-                        // AND didn't request legacy storage to be preserved
-                        if (!hasWriteMediaStorageGrantedForUid
-                                && !hasRequestedPreserveLegacyExternalStorage) {
+                        // The package doesn't request legacy storage to be preserved
+                        if (!hasRequestedPreserveLegacyExternalStorage) {
                             return true;
                         }