CTRL-R can cause accidental code injection through fzf --query flag in shell scripts

[chrisant996]

• I have read through the manual page (man fzf)
• I have the latest version of fzf
• I have searched through the existing issues

Info

• OS
  • Linux
  • Mac OS X
  • Windows
  • Etc.
• Shell
  • bash
  • zsh
  • fish

Problem / Steps to reproduce

The way that the --query flag is used has a code injection issue.

In different shells, different kinds of input characters can cause part of the --query string to get misinterpreted by the shell as another command.

This was first noticed with cmd.exe + clink and clink-fzf, which contains a Lua script for integrating fzf with Clink.

If the --query argument string is not quoted, then text like & echo hello can result in the echo hello part of the query text getting interpreted as a separate shell command, instead of as a query string.

If the --query argument string is quoted, then text like exe" & echo hello can result in the echo hello part getting interpreted as a separate shell command, instead of as a query string.

If the echo hello in the example is replaced by other more dangerous text, then very unfortunate accidental side effect could occur, just from trying to search in the command history. For example, in cmd.exe rd /s would recursively removes all directories.

This is fixed for cmd.exe + clink by chrisant996/clink-fzf@5415363.

Other shells could likely use a similar approach of stripping certain problematic characters (especially quotes) and surrounding the reduced query string in quotes. The specific set of problematic characters is different for different shells.

[junegunn]

Thanks for the heads up.

I don't see this as a problem. If this is a problem, then we're saying that every command-line program that takes an arbitrary string as an argument has the problem. i.e. grep, sed, awk, to name a few.

When a script or a program is invoking a shell command, it is its responsibility to properly escape the arguments. You can find libraries for that purpose in many programming languages. They're usually called shellescape.

• https://ruby-doc.org/3.2.1/stdlibs/shellwords/Shellwords.html#method-c-shellescape
• https://vimhelp.org/builtin.txt.html#shellescape%28%29
• https://pypi.org/project/shellescape/

[chrisant996]

I don't see this as a problem. If this is a problem, then we're saying that every command-line program that takes an arbitrary string as an argument has the problem. i.e. grep, sed, awk, to name a few.

@junegunn No, this is very different from grep/etc because CTRL-R automatically passes the raw input line to fzf --query without escaping or sanitizing the input line first.

Depending on what the user has typed into the shell input line so far, pressing CTRL-R can unwittingly trigger code injection and mistakenly execute part of the input line.

I meant that CTRL-R's usage of --query can trigger a code injection problem. Sorry for being unclear initially.

The fix for CMD involves stripping ", ^, and % first and then surrounding the input line with quotes. Those are the problem characters with cmd.exe, but different shells will require different sanitization strategies. And because fzf uses fuzzy matching, the sanitized query string is still able to do a good job of filtering.

[chrisant996]

@junegunn The issue is with how shells scripts invoke --query. For example here. Pressing CTRL-R can accidentally execute part of the input line by mistake.

[junegunn]

The code is perfectly fine and safe because the varible reference is quoted. bash and zsh don't have the problem you describe. Maybe quoting is not good enough in cmd.exe, but this repository doesn't provide any cmd.exe script.

[chrisant996]

I see. Bash explicitly avoids parsing what's inside the expanded "$variable". If that's the case for all supported shells, then I agree there shouldn't be a code injection issue.

I don't expect fzf to support cmd.exe. But for context, cmd.exe expands variables and then parses the resulting command line. It isn't my intent to debate pros/cons of different shell designs; I only want to help ensure safety in each shell. The external integration scripts for fzf and cmd.exe handle this safely in a cmd.exe-specific way.

Thanks!

[chrisant996]

Follow-up for those who are curious about the details how this works

I couldn't find any documentation explaining how quoting a shell variable works. It seemed strangely complicated and problematic for bash to be able to avoid parsing the expanded variable, and my curiosity was piqued. And indeed, that's not how it works.

So I tracked it down.

Short version

Bash automatically applies escaping when a variable expansion happens within double quotes such as "$variable".

Cmd does not automatically apply escaping; it must be done manually.

I wasn't able to find anything that actually stated that bash applies escaping when variable expansion happens within double quotes; I had to determine it empirically. Presumably I just didn't know the right search terms to find it.

Long version

I'm using WSL (Windows subsystem for Linux), so each separate terminal is a separate instance of Linux. They're unaware of each other, so I couldn't use ps to find how bash passes the command line to fzf. I tried looking in bash source code for the variable expansion code, but finding the code was turning into a time sink.

So I took an "easy way out" and copied the __fzf_history__ function, made a new key binding, and made it spawn a Windows .exe file into. Then I was able to use Process Explorer to observe the command line.

How variable quoting works in bash

When bash expands "$variable", it automatically internally escapes double quotes by translating them to \". It also escapes \ into \\, and presumably it might also escape some other characters as well.

When parsing a command line, within a double quoted run \" is interpreted as an escaped double quote, and is translated back to " in output from the parse. (And \\ and maybe some other characters.) The surround double quotes are of course discarded during parsing.

The actual command line passed to the spawned process contains both the outer quotes and the escaped inner quotes, for example "abc\" 123 \"xyz".

How command lines are handled on Windows

This is not meant as a critique of how Windows works, nor why it works that way, it is just an explanation of how it is. There are historical reasons why, and in the context of the time they were made, most of them actually make sense as pragmatic trade-offs. (Tracking down all the historical reasons is left as as exercise for the reader.)

Synopsis:

• On Windows \ is the path separator, so it can't universally serve as an escape character. The decision was originally motivated by the need for CP/M compatibility where / is the switch character.
• On Windows the program startup code for parsing a command line has special parsing (see here and here and here and here) to handle un-escaped quotes and un-escaped backslash, and within un-escaped quotes it also handles escaped quotes and certain instances of escaped backslash. This approach has quirks in certain edge cases, and because they only manifest in edge cases, everyone forgets they exist (or doesn't even know they exist).
• On Windows un-escaped double quotes in the command line are stripped, just like on *nix.
• On Windows programs receive an unparsed command line, and the C runtime (or other language runtime) parses it into an array argv[] of arguments. The original unparsed command line is also available via GetCommandLine().
• On Windows there is also an API CommandLineToArgvW() which can parse a UTF16 command line into UTF16 argv[]. Whether any particular language's startup code uses GetCommandLineToArgvW() or uses custom code or uses the C runtime (if the language itself is implemented in C) is an implementation detail of the language.

Knowing how command line parsing working on Windows, we would expect the following:

fzf --query "abc\" 123 \"xyz" should yield

• argv[0] = fzf
• argv[1] = --query
• argv[2] = abc" 123 "xyz

And generally speaking, it does, if an executable program (a .exe file) is spawned. But if it spawns a cmd shell script, then cmd will parse the command line differently, and things can get tricky. That's just part of cmd shell scripting, but the trouble is the caller has to format the command differently depending on whether it will be parsed by a cmd shell script, or by an executable program.

Conclusion

Bash automatically applies escaping when a variable expansion happens within double quotes such as "$variable".

Cmd does not automatically apply escaping; it must be done manually. On Windows, it should be possible to explicitly invoke fzf.exe rather than fzf (to avoid parsing issues with a fzf.bat that might intercept the command), and to quote the --query string, and within the query string to apply escaping to " (and sometimes to \ according to the 2N rule described in the four "here" links cited earlier).