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
This commit is contained in:
Ivan Chiang
2022-04-19 09:29:44 +00:00
parent bd72a95c1f
commit 8b55dd05ce
3 changed files with 43 additions and 12 deletions

View File

@@ -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)) {

View File

@@ -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);
}

View File

@@ -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);
}
}