Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize parameters to prevent path injection #3935

Merged
merged 2 commits into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public static OutputStream getOutputStream(final File target, Context context)
/** Writes uri stream from external application to the specified path */
public static final void writeUriToStorage(
@NonNull final MainActivity mainActivity,
@NonNull final ArrayList<Uri> uris,
@NonNull final List<Uri> uris,
@NonNull final ContentResolver contentResolver,
@NonNull final String currentPath) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package com.amaze.filemanager.filesystem.root

import android.os.Build
import com.amaze.filemanager.fileoperations.exceptions.ShellNotRunningException
import com.amaze.filemanager.filesystem.RootHelper
import com.amaze.filemanager.filesystem.root.base.IRootCommand

object MountPathCommand : IRootCommand() {
Expand All @@ -38,7 +39,8 @@ object MountPathCommand : IRootCommand() {
* @return String the root of mount point that was ro, and mounted to rw; null otherwise
*/
@Throws(ShellNotRunningException::class)
fun mountPath(path: String, operation: String): String? {
fun mountPath(pathArg: String, operation: String): String? {
val path = RootHelper.getCommandLineString(pathArg)
return when (operation) {
READ_WRITE -> mountReadWrite(path)
READ_ONLY -> {
Expand Down Expand Up @@ -96,7 +98,9 @@ object MountPathCommand : IRootCommand() {
return if (mountOutput.isNotEmpty()) {
// command failed, and we got a reason echo'ed
null
} else mountPoint
} else {
mountPoint
}
}
}
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public class MainActivity extends PermissionsActivity
public ArrayList<String> oppatheList;

// This holds the Uris to be written at initFabToSave()
private ArrayList<Uri> urisToBeSaved;
private List<Uri> urisToBeSaved;

public static final String PASTEHELPER_BUNDLE = "pasteHelper";

Expand Down Expand Up @@ -630,9 +630,15 @@ private void checkForExternalIntent(Intent intent) {
} else {
// save a single file to filesystem
Uri uri = intent.getParcelableExtra(Intent.EXTRA_STREAM);
ArrayList<Uri> uris = new ArrayList<>();
uris.add(uri);
initFabToSave(uris);
if (uri != null
&& uri.getScheme() != null
&& uri.getScheme().startsWith(ContentResolver.SCHEME_FILE)) {
ArrayList<Uri> uris = new ArrayList<>();
uris.add(uri);
initFabToSave(uris);
} else {
Toast.makeText(this, R.string.error_unsupported_or_null_uri, Toast.LENGTH_LONG).show();
}
}
// disable screen rotation just for convenience purpose
// TODO: Support screen rotation when saving a file
Expand All @@ -651,7 +657,7 @@ private void checkForExternalIntent(Intent intent) {
}

/** Initializes the floating action button to act as to save data from an external intent */
private void initFabToSave(final ArrayList<Uri> uris) {
private void initFabToSave(final List<Uri> uris) {
Utils.showThemedSnackbar(
this,
getString(R.string.select_save_location),
Expand All @@ -660,7 +666,7 @@ private void initFabToSave(final ArrayList<Uri> uris) {
() -> saveExternalIntent(uris));
}

private void saveExternalIntent(final ArrayList<Uri> uris) {
private void saveExternalIntent(final List<Uri> uris) {
executeWithMainFragment(
mainFragment -> {
if (uris != null && uris.size() > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,18 @@ class CompressedExplorerFragment : Fragment(), BottomBarButtonPath {
)?.run {
try {
if (moveToFirst()) {
fileName = getString(0)
fileName = getString(0).let {
/*
* Strip any slashes to prevent possibility to access files outside
* cache dir if malicious ContentProvider gives malicious value
* of MediaStore.MediaColumns.DISPLAY_NAME when querying
*/
if (it.contains(File.pathSeparator)) {
it.substringAfterLast(File.pathSeparatorChar)
} else {
it
}
}
compressedFile = File(requireContext().cacheDir, fileName)
} else {
// At this point, we know nothing the file the URI represents, we are doing everything
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -813,5 +813,6 @@ You only need to do this once, until the next time you select a new location for
<string name="try_indexed_search">Try Indexed Search!</string>
<string name="search_recent">Recent</string>
<string name="results">Results</string>
<string name="error_unsupported_or_null_uri">Invalid, unsupported, or no URI was provided</string>
</resources>