Block subdirs of Android for SAF and normalize the path

- When the Apps launch SAF, they can set initial uri to launch the
  specific directory. Block the access for data/obb/sandbox
  directories of Android
- Normalize path (E.g. the path includes . or ..) to avoid
  unexpected behavior

Bug: 200034476
Bug: 220066255
Test: atest ExternalStorageProviderTest
Test: atest DocumentsTest
Change-Id: I226a9c25710fcf78d14d775eb3270bfd3b491dd3

3 files changed, 43 insertions, 12 deletions

diff --git a/core/java/com/android/internal/content/FileSystemProvider.java b/core/java/com/android/internal/content/FileSystemProvider.java
index a60b31078a86..563cf0414ab9 100644
--- a/core/java/com/android/internal/content/FileSystemProvider.java
+++ b/core/java/com/android/internal/content/FileSystemProvider.java
@@ -414,6 +414,12 @@ public abstract class FileSystemProvider extends DocumentsProvider {
         final File parent = getFileForDocId(parentDocumentId);
         final MatrixCursor result = new DirectoryCursor(
                 resolveProjection(projection), parentDocumentId, parent);
+
+        if (!filter.test(parent)) {
+            Log.w(TAG, "No permission to access parentDocumentId: " + parentDocumentId);
+            return result;
+        }
+
         if (parent.isDirectory()) {
             for (File file : FileUtils.listFilesOrEmpty(parent)) {
                 if (filter.test(file)) {
diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
index ce58ff6fc59d..4c313b22f71e 100644
--- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
+++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
@@ -61,6 +61,7 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.UUID;
 
@@ -318,13 +319,19 @@ public class ExternalStorageProvider extends FileSystemProvider {
             }
 
             // Block Download folder from tree
-            if (TextUtils.equals(Environment.DIRECTORY_DOWNLOADS.toLowerCase(),
-                    path.toLowerCase())) {
+            if (TextUtils.equals(Environment.DIRECTORY_DOWNLOADS.toLowerCase(Locale.ROOT),
+                    path.toLowerCase(Locale.ROOT))) {
                 return true;
             }
 
-            if (TextUtils.equals(Environment.DIRECTORY_ANDROID.toLowerCase(),
-                    path.toLowerCase())) {
+            // Block /Android
+            if (TextUtils.equals(Environment.DIRECTORY_ANDROID.toLowerCase(Locale.ROOT),
+                    path.toLowerCase(Locale.ROOT))) {
+                return true;
+            }
+
+            // Block /Android/data, /Android/obb, /Android/sandbox and sub dirs
+            if (shouldHide(dir)) {
                 return true;
             }
 
@@ -420,19 +427,21 @@ public class ExternalStorageProvider extends FileSystemProvider {
     }
 
     @VisibleForTesting
-    static String getPathFromDocId(String docId) {
+    static String getPathFromDocId(String docId) throws IOException {
         final int splitIndex = docId.indexOf(':', 1);
-        final String path = docId.substring(splitIndex + 1);
+        final String docIdPath = docId.substring(splitIndex + 1);
+        // Get CanonicalPath and remove the first "/"
+        final String canonicalPath = new File(docIdPath).getCanonicalPath().substring(1);
 
-        if (path.isEmpty()) {
-            return path;
+        if (canonicalPath.isEmpty()) {
+            return canonicalPath;
         }
 
         // remove trailing "/"
-        if (path.charAt(path.length() - 1) == '/') {
-            return path.substring(0, path.length() - 1);
+        if (canonicalPath.charAt(canonicalPath.length() - 1) == '/') {
+            return canonicalPath.substring(0, canonicalPath.length() - 1);
         } else {
-            return path;
+            return canonicalPath;
         }
     }
 
@@ -463,7 +472,12 @@ public class ExternalStorageProvider extends FileSystemProvider {
         if (!target.exists()) {
             target.mkdirs();
         }
-        target = new File(target, path);
+        try {
+            target = new File(target, path).getCanonicalFile();
+        } catch (IOException e) {
+            throw new FileNotFoundException("Failed to canonicalize path " + path);
+        }
+
         if (mustExist && !target.exists()) {
             throw new FileNotFoundException("Missing file for " + docId + " at " + target);
         }
diff --git a/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java b/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
index ed8320fa7fef..18a8edc5e447 100644
--- a/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
+++ b/packages/ExternalStorageProvider/tests/src/com/android/externalstorage/ExternalStorageProviderTest.java
@@ -67,5 +67,16 @@ public class ExternalStorageProviderTest {
 
         docId = root + ":";
         assertTrue(getPathFromDocId(docId).isEmpty());
+
+        docId = root + ":./" + path;
+        assertEquals(getPathFromDocId(docId), path);
+
+        final String dotPath = "abc/./def/ghi";
+        docId = root + ":" + dotPath;
+        assertEquals(getPathFromDocId(docId), path);
+
+        final String twoDotPath = "abc/../abc/def/ghi";
+        docId = root + ":" + twoDotPath;
+        assertEquals(getPathFromDocId(docId), path);
     }
 }