fd is vulnerable to filenames with leading dashes

[ykonstant1]

Describe the bug you encountered: When piping relative paths from fd to utilities like xargs, the program does not return paths, but rather file names. For instance, in a directory containing foo.txt, the command fd -t f -0 | xargs -0 -I% rm % passes foo.txt to xargs rather than ./foo.txt.

If the directory contains files with leading dashes, such as the file -rf, commands like rm interpret this as options rather than a valid path. Certain commands have the end-of-options -- parameter, but not all do, and the burden falls on the user to use --. find fixes this problem by always returning absolute or relative paths; returning ./-rf would not pose a problem for further processing.

Filenames with dashes are not limited to deliberately malicious constructs. I have found program-generated files in .config folders full of leading dashes, as well as media files saved by id, whose id happened to have a leading dash.

Describe what you expected to happen: when searching by relative paths, fd should return paths, i.e. filenames prefixed by ./ to avoid problems like the above; especially when the output of fd is in a pipeline.

What version of fd are you using? fd 8.2.1

Which operating system / distribution are you on?
Linux 5.11.12-arch1-1 x86_64

LSB Version: 1.4
Distributor ID: Arch
Description: Arch Linux
Release: rolling
Codename: n/a

[tmccombs]

You could use the --absolute-path or -a option to have fd output full paths instead of just the filename.

[ykonstant1]

Certainly; the problem is with a third user who may be unaware of the leading dash issue. If fd does not want to return relative paths, should the documentation emphasize to always use -a when using fd in a pipe, or even enforce -a in such a case?

[sharkdp]

Thank you for reporting this. I agree that we should fix this somehow. Unfortunately, this will cause breaking changes, as some people probably rely on the current format of passing arguments (I certainly do).

Using absolute paths would be an easy way out, but I don't think it's desirable, especially for commands where the user sees the paths. I'd rather try to follow find here.

[wodny]

Certainly; the problem is with a third user who may be unaware of the leading dash issue. If fd does not want to return relative paths, should the documentation emphasize to always use -a when using fd in a pipe, or even enforce -a in such a case?

BTW, in such cases I like to link to a superb article by David A. Wheeler about dangerous filenames and managing the problem. If there is place for a "further reading" link I propose to add this one.

[sharkdp]

The same problem actually appears with fds builtin -x/-X as well.

how to (safely) reproduce:

> mkdir /tmp/issue-760; cd /tmp/issue-760
> touch -- '-e' 'foo.txt'
> fd -X echo
foo.txt                                        # <-- should include '-e' as well
> fd -0 | xargs -0 echo
foo.txt                                        # <-- same

> fd -a -X echo # workaround
/tmp/issue-760/-e /tmp/issue-760/foo.txt

I think we could fix this by:

• prepending ./ for all relative paths when using -x/-X.
• prepending ./ for all relative paths when the output is a non-interactive terminal

This way, we can leave the normal (interactive console) fd output unmodified.

[Phrynobatrachus]

I'm messing around with some changes for this, The same behavior should apply to Windows, assuming it's not a path that already starts with a Component::Prefix, right?

[sharkdp]

I'm messing around with some changes for this, The same behavior should apply to Windows, assuming it's not a path that already starts with a Component::Prefix, right?

yes. whenever it's a relative path (I guess). Might make sense to validate that the same bug appears on Windows as well.

[Phrynobatrachus]

I'm not too familiar with Windows command line stuff so I might be doing something wrong, but fd -X echo outputs 'command not found', even though plain echo whatever does work (same on both cmd and powershell).* Other than that, I think all that would need to change is the filesystem::strip_current_dir calls during command generation and when printing.

#538 describes what I was missing, will try testing some more*

[jzinn]

I think we could fix this by:

* prepending `./` for all relative paths when using `-x`/`-X`.

* prepending `./` for all relative paths when the output is a non-interactive terminal

Prepending ./ to all relative paths does not match find's behavior.

This is my test tree:

% tree
.
├── FOO
└── a
    └── FOO

1 directory, 2 files

These are my test commands:

% find . -name '*O*' 
./a/FOO
./FOO

% find a -name '*O*'
a/FOO

% find . a -name '*O*'
./a/FOO
./FOO
a/FOO

% find -name '*O*' 
./a/FOO
./FOO

If a is a search path, then the output includes a/FOO, not ./a/FOO.

If no search path is provided, then the search path is .. This is likely what leads to the search results having a ./ prefix, and find probably does not do an extra explicit prepend operation of ./ to relative paths.

Perhaps this find example (repeated from above)

% find a -name '*O*'
a/FOO

should instead do this:

% find a -name '*O*'
./a/FOO

Should fd should follow find here? If someone deliberately names a file with a leading dash, will find's behavior be preferable for some cases? I'm not sure. Maybe unconditionally doing an extra prepend operation of ./ to all relative paths that don't already have it is best. I can't decide.

Off topic

I am currently writing a script for reposurgeon that renames a bunch of files and directories to names like

-XXX-package-lock.json-haiw9Ohphae

and

-XXX-.git-haiw9Ohphae

(People accidentally check in .git directories to Subversion repositories.)

I picked dash as the leading character because it has the lowest reasonable ASCII value that should belong in a file name, and therefore the files sort before all others, including . and ..:

% ls -a1
-XXX-package-lock.json-haiw9Ohphae
.
..
other.files

I didn't think about the xargs vulnerability!

Though the "random" string, above haiw9Ohphae (pwgen 11 ftw!), should ensure that no command ever interprets it as a useful flag, perhaps I should pick a different leading character.

[tavianator]

Should fd should follow find here? If someone deliberately names a file with a leading -, will find's behavior be preferable for some cases? I'm not sure. Maybe prepending ./ unconditionally to relative paths is best. I can't decide.

It is tough to get find to search in a directory named with a leading dash in the first place:

$ mkdir -- -foo
$ find -foo
find: unknown predicate `-foo'
$ find -- -foo
find: unknown predicate `-foo'

(Though it can be done with BSD's find -f -foo or GNU find's upcoming printf '-foo' | find -files0-from -.)

[jzinn]

It is tough to get find to search in a directory named with a leading dash in the first place:

I didn't know that. I will experiment.

I guess I was wondering which is more desirable, this actual output:

$ mkdir -- -foo
$ touch -- -foo/bar
$ fd a | cat
-foo/bar

or this imaginary output:

$ fd a | cat
./-foo/bar

[jzinn]

It is tough to get find to search in a directory named with a leading dash in the first place:

$ mkdir -- -foo
$ find -foo
find: unknown predicate `-foo'
$ find -- -foo
find: unknown predicate `-foo'

I didn't know that. I will experiment.

Apparently this is the way to do it:

$ find ./-foo
./-foo
./-foo/bar

This was helpful to figure that out:

https://dwheeler.com/essays/filenames-in-shell.html

[jzinn]

For reference:

https://dwheeler.com/essays/fixing-unix-linux-filenames.html#dashes

[jzinn]

I picked - as a leading character because it has the lowest reasonable ASCII value that should belong in a file name, and therefore the files sort before all others, including . and ..:

% ls -a1
-XXX-package-lock.json-haiw9Ohphae
.
..
other.files

perhaps I should pick a different leading character

@david-a-wheeler says

Globs are expanded by the shell into a list of filenames, and dash is earlier in the sort order compared to before alphanumerics, so it is easy for attackers to make this happen.

[jzinn]

It would be quite pleasing for fd to work this way:

$ mkdir a
$ touch FOO a/FOO

$ fd O . a
FOO
a/FOO
a/FOO

$ fd O . a | cat
./FOO
./a/FOO
./a/FOO

$ fd O . ./a
FOO
a/FOO
a/FOO

$ fd O . ./a | cat
./FOO
./a/FOO
./a/FOO

[sharkdp]

@jzinn Maybe you could try the version from the feature branch in #861 and report if something works not as expected?

[jzinn]

These seem to be working:

% /home/me/src/git/fd/target/debug/fd O            
FOO
a/FOO
% /home/me/src/git/fd/target/debug/fd O | cat
./FOO
./a/FOO

These, IMO, are not working:

% /home/me/src/git/fd/target/debug/fd O . a        
./FOO
./a/FOO
a/FOO
% /home/me/src/git/fd/target/debug/fd O . a | cat
./FOO
./a/FOO
a/FOO
% /home/me/src/git/fd/target/debug/fd O . ./a    
./FOO
./a/FOO
./a/FOO
% /home/me/src/git/fd/target/debug/fd O . ./a | cat
./FOO
./a/FOO
./a/FOO

% /home/me/src/git/fd/target/debug/fd --version    
fd 8.3.0

Still vulnerable?

Test tree:

% mkdir -- -foo
% touch -- -foo/FOO
% tree
.
└── -foo
    └── FOO

Actual:

% /home/me/src/git/fd/target/debug/fd O -- -foo | cat
-foo/FOO

% /home/me/src/git/fd/target/debug/fd O -- * | cat   
-foo/FOO

Expected:

% /home/me/src/git/fd/target/debug/fd O -- -foo | cat
./-foo/FOO

% /home/me/src/git/fd/target/debug/fd O -- * | cat   
./-foo/FOO

[jzinn]

Maybe it is working as expected, it's not for me to decide.

Files

The most important case, file names with leading dashes, is covered in this actual output:

% tree
.
└── -FOO
% /home/me/src/git/fd/target/debug/fd O           
-FOO
% /home/me/src/git/fd/target/debug/fd O | cat
./-FOO
% /home/me/src/git/fd/target/debug/fd O .    
./-FOO
% /home/me/src/git/fd/target/debug/fd O . | cat
./-FOO

though my preferred expected output would be:

% /home/me/src/git/fd/target/debug/fd O           
-FOO
% /home/me/src/git/fd/target/debug/fd O | cat
./-FOO
% /home/me/src/git/fd/target/debug/fd O .    
-FOO
% /home/me/src/git/fd/target/debug/fd O . | cat
./-FOO

Directories

Is it worth worrying about directory names with leading dashes? Maybe not, but it seems like the next logical step to me.

And, as I mentioned in an earlier comment, the following would be my preferred expected output, and I haven't (so far) thought of any downsides:

% tree
.
├── FOO
└── a
    └── FOO
% /home/me/src/git/fd/target/debug/fd O
FOO
a/FOO
% /home/me/src/git/fd/target/debug/fd O | cat
./FOO
./a/FOO
% /home/me/src/git/fd/target/debug/fd O . a        
FOO
a/FOO
a/FOO
% /home/me/src/git/fd/target/debug/fd O . a | cat
./FOO
./a/FOO
./a/FOO
% /home/me/src/git/fd/target/debug/fd O . ./a    
FOO
a/FOO
a/FOO
% /home/me/src/git/fd/target/debug/fd O . ./a | cat
./FOO
./a/FOO
./a/FOO

[jzinn]

I haven't (so far) thought of any downsides

I suppose if you're scripting and you do something like

while read i
do
   fd ... "$i" ... | xargs process "$i"
done

then you might want fd to do exactly what you request, as find does, and not prepend or strip any leading ./.

Perhaps the --strip-cwd-prefix option enables this. If fd behavior changes, perhaps this flag should be changed to --modify-cwd-prefix and --no-modify-cwd-prefix.

[jzinn]

I think the goals of this issue have been achieved, and my preferred expected output should probably be a separate issue.

With no search path specified, the output is protected:

% /home/me/src/git/fd/target/debug/fd O | cat       
./-foo/FOO

When the search path is specified, then the output is not protected, but the output is what is explicitly requested:

% /home/me/src/git/fd/target/debug/fd O -- * | cat  
-foo/FOO

And the output can be explicitly protected as well, by prefixing the search path with ./:

% /home/me/src/git/fd/target/debug/fd O -- ./* | cat
./-foo/FOO

[sharkdp]

Ok, thank you for the summary. I think I would consider this closed indeed.