Skip to content

Commit

Permalink
[Refactor] Sanitise user supplied file paths
Browse files Browse the repository at this point in the history
User-supplied file paths should always be absolute URIs. Otherwise, App Manager
will try to resolve an absolute path from the given path or fail.

Signed-off-by: Muntashir Al-Islam <muntashirakon@riseup.net>
  • Loading branch information
MuntashirAkon committed Aug 24, 2023
1 parent ee64ce8 commit f6a77fc
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ public void onItemRangeChanged(int positionStart, int itemCount) {
pathListView.setAdapter(mPathListAdapter);
MaterialButton pathEditButton = view.findViewById(R.id.uri_edit);
pathEditButton.setOnClickListener(v -> {
String path = FmUtils.getDisplayablePath(mModel.getCurrentUri());
Uri currentUri = mModel.getCurrentUri();
String path = currentUri != null ? FmUtils.getDisplayablePath(currentUri) : null;
new TextInputDialogBuilder(mActivity, null)
.setTitle(R.string.go_to_path)
.setInputText(path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static Uri getContentUri(@NonNull Path path) {
return getContentUri(path.getUri());
}

@VisibleForTesting
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
@NonNull
static Uri getContentUri(@NonNull Uri uri) {
Uri.Builder builder = uri.buildUpon()
Expand Down Expand Up @@ -215,7 +215,7 @@ private static Path getFileProviderPath(@NonNull Uri uri) throws FileNotFoundExc
*
* @see #getContentUri(Uri)
*/
@VisibleForTesting
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
@NonNull
static Uri getFileProviderPathInternal(@NonNull Uri uri) {
List<String> pathParts = uri.getPathSegments();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.provider.DocumentsContract;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import java.io.File;
import java.util.ArrayList;
Expand All @@ -19,6 +20,7 @@

import io.github.muntashirakon.AppManager.utils.ContextUtils;
import io.github.muntashirakon.io.Path;
import io.github.muntashirakon.io.Paths;
import io.github.muntashirakon.io.fs.VirtualFileSystem;

public final class FmUtils {
Expand Down Expand Up @@ -60,6 +62,54 @@ private static String getSingleMode(int mode, boolean special, String specialCha
execMode;
}


@Nullable
public static Uri sanitizeContentInput(@Nullable Uri uri) {
if (uri == null) {
return null;
}
// App Manager supports three schemes: file, content and vfs (private)
String scheme = uri.getScheme();
if (scheme == null) {
// Scheme must be non-null unless explicitly required (such as in FmActivity)
return null;
}
switch (scheme) {
case ContentResolver.SCHEME_CONTENT:
// Content schemes are handled by authority.
if (FmProvider.AUTHORITY.equals(uri.getAuthority())) {
// Sanitize the path part which must be an absolute URL
Uri realUri = FmProvider.getFileProviderPathInternal(uri);
Uri fixedUri = sanitizeContentInput(realUri);
return fixedUri != null ? FmProvider.getContentUri(fixedUri) : null;
}
// If it's not App Manager itself, return as is.
return uri;
case ContentResolver.SCHEME_FILE: {
// Sanitize the path which must be an absolute URL
String path = uri.getPath();
path = Paths.relativePath(path, File.separator);
return uri.buildUpon().path(path).build();
}
case VirtualFileSystem.SCHEME: {
if (uri.getAuthority() == null) {
// VFS must have an authority
return null;
}
// VFS path must be absolute URL
String path = uri.getPath();
if (!path.startsWith(File.separator)) {
path = File.separator + path;
}
path = Paths.relativePath(path, File.separator);
return uri.buildUpon().path(path).build();
}
default:
// Invalid path
return null;
}
}

@SuppressWarnings("SuspiciousRegexArgument") // We're not on Windows
public static List<String> uriToPathParts(@NonNull Uri uri) {
switch (uri.getScheme()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.StringTokenizer;

import io.github.muntashirakon.AppManager.compat.IntegerCompat;
import io.github.muntashirakon.AppManager.fm.FmUtils;
import io.github.muntashirakon.AppManager.utils.MotorolaUtils;

public final class IntentCompat {
Expand Down Expand Up @@ -84,21 +85,29 @@ public static Uri getDataUri(@Nullable Intent intent) {
return null;
}
if (Intent.ACTION_SEND.equals(intent.getAction())) {
return getParcelableExtra(intent, Intent.EXTRA_STREAM, Uri.class);
return FmUtils.sanitizeContentInput(getParcelableExtra(intent, Intent.EXTRA_STREAM, Uri.class));
}
return intent.getData();
return FmUtils.sanitizeContentInput(intent.getData());
}

@Nullable
public static List<Uri> getDataUris(@NonNull Intent intent) {
if (Intent.ACTION_SEND.equals(intent.getAction())) {
return Collections.singletonList(getParcelableExtra(intent, Intent.EXTRA_STREAM, Uri.class));
} else if (Intent.ACTION_SEND_MULTIPLE.equals(intent.getAction())) {
return getParcelableArrayListExtra(intent, Intent.EXTRA_STREAM, Uri.class);
if (Intent.ACTION_SEND_MULTIPLE.equals(intent.getAction())) {
List<Uri> inputUris = getParcelableArrayListExtra(intent, Intent.EXTRA_STREAM, Uri.class);
if (inputUris == null) {
return null;
}
List<Uri> filteredUris = new ArrayList<>(inputUris.size());
for (Uri uri : inputUris) {
Uri fixedUri = FmUtils.sanitizeContentInput(uri);
if (fixedUri != null) {
filteredUris.add(fixedUri);
}
}
return filteredUris.isEmpty() ? null : filteredUris;
}
Uri data = intent.getData();
if (data == null) return null;
return Collections.singletonList(data);
Uri uri = getDataUri(intent);
return uri != null ? Collections.singletonList(uri) : null;
}

public static void removeFlags(@NonNull Intent intent, int flags) {
Expand Down Expand Up @@ -693,25 +702,26 @@ public static String toUri(@NonNull Intent intent, int flags) {
uri.append("android-app://");
uri.append(intent.getPackage());
String scheme = null;
if (intent.getData() != null) {
scheme = intent.getData().getScheme();
Uri data = intent.getData();
if (data != null) {
scheme = data.getScheme();
if (scheme != null) {
uri.append('/');
uri.append(scheme);
String authority = intent.getData().getEncodedAuthority();
String authority = data.getEncodedAuthority();
if (authority != null) {
uri.append('/');
uri.append(authority);
String path = intent.getData().getEncodedPath();
String path = data.getEncodedPath();
if (path != null) {
uri.append(path);
}
String queryParams = intent.getData().getEncodedQuery();
String queryParams = data.getEncodedQuery();
if (queryParams != null) {
uri.append('?');
uri.append(queryParams);
}
String fragment = intent.getData().getEncodedFragment();
String fragment = data.getEncodedFragment();
if (fragment != null) {
uri.append('#');
uri.append(fragment);
Expand All @@ -724,8 +734,9 @@ public static String toUri(@NonNull Intent intent, int flags) {
return uri.toString();
}
String scheme = null;
if (intent.getData() != null) {
String data = intent.getData().toString();
Uri dataUri = intent.getData();
if (dataUri != null) {
String data = dataUri.toString();
if ((flags & Intent.URI_INTENT_SCHEME) != 0) {
final int N = data.length();
for (int i = 0; i < N; i++) {
Expand Down Expand Up @@ -767,8 +778,8 @@ private static void toUriFragment(Intent intent, StringBuilder uri, @Nullable St
// Note that for now we are not going to try to handle the
// data part; not clear how to represent this as a URI, and
// not much utility in it.
toUriInner(intent.getSelector(), frag, intent.getSelector().getData() != null
? intent.getSelector().getData().getScheme() : null, null, null, flags);
Uri data = intent.getSelector().getData();
toUriInner(intent.getSelector(), frag, data != null ? data.getScheme() : null, null, null, flags);
}

if (frag.length() > 0) {
Expand Down

0 comments on commit f6a77fc

Please sign in to comment.