I'm writing a GUI application for character recognition that uses Tesseract. I want to allow the user to specify a custom shell command to be executed with /bin/sh -c
when the text is ready.
The problem is the recognized text can contain literally anything, for example && rm -rf some_dir
.
My first thought was to make it like in many other programs, where
the user can type the command in a text entry, and then special strings (like in printf()
) in the command are replaced by the appropriate data (in my case, it might be %t
). Then the whole string is passed to execvp()
. For example, here is a screenshot from qBittorrent:
The problem is that even if I properly escape the text before replacing %t
, nothing prevents the user to add extra quotes around the specifier:
echo '%t' >> history.txt
So the full command to be executed is:
echo ''&& rm -rf some_dir'' >> history.txt
Obviously, that's a bad idea.
The second option is only let the user to choose an executable (with a file selection dialog), so I can manually put the text from Tesseract as argv[1]
for execvp()
. The idea is that the executable can be a script where users can put anything they want and access the text with "$1"
. That way, the command injection is not possible (I think). Here's an example script a user can create:
#!/bin/sh
echo "$1" >> history.txt
It there any pitfalls with this approach? Or maybe there's a better way to safely pass an arbitrary text as parameter to a program in shell script?
Don't do this. See the "Out-Of-Band" section below.
To make an arbitrarily C string (containing no NULs) evaluate to itself when used in an unquoted context in a strictly POSIX-compliant shell, you can use the following steps:
'
(moving from the required initial unquoted context to a single-quoted context).'
within the data with the string '"'"'
. These characters work as follows:
'
closes the initial single-quoted context."
enters a double-quoted context.'
is, in a double-quoted context, literal."
closes the double-quoted context.'
re-enters single-quoted context.'
(returning to the required initial single-quoted context).This works correctly in a POSIX-compliant shell because the only character that is not literal inside of a single-quoted context is '
; even backslashes are parsed as literal in that context.
However, this only works correctly when sigils are used only in an unquoted context (thus putting onus on your users to get things right), and when a shell is strictly POSIX-compliant. Also, in a worst-case scenario, you can have the string generated by this transform be up to 5x longer than the original; one thus needs to be cautious around how the memory used for the transform is allocated.
(One might ask why '"'"'
is advised instead of '\''
; this is because backslashes change their meaning used inside legacy backtick command substitution syntax, so the longer form is more robust).
Data should only be passed out-of-band from code, such that it's never run through the parser at all. When invoking a shell, there are two straightforward ways to do this (other than using files): Environment variables, and command-line arguments.
In both of the below mechanisms, only the user_provided_shell_script
need be trusted (though this also requires that it be trusted not to introduce new or additional vulnerabilities; invoking eval
or any moral equivalent thereto voids all guarantees, but that's the user's problem, not yours).
Excluding error handling (if setenv()
returns a nonzero result, this should be treated as an error, and perror()
or similar should be used to report to the user), this will look like:
setenv("torrent_name", torrent_name_str, 1);
setenv("torrent_category", torrent_category_str, 1);
setenv("save_path", path_str, 1);
# shell script should use "$torrent_name", etc
system(user_provided_shell_script);
A few notes:
execve()
-family calls with large command lines to fail. Validating that length is within reasonable domain-specific limits is wise.This version requires an explicit API, such that the user configuring the trigger command knows which value will be passed in $1
, which will be passed in $2
, etc.
/* You'll need to do the usual fork() before this, and the usual waitpid() after
* if you want to let it complete before proceeding.
* Lots of Q&A entries on the site already showing the context.
*/
execl("/bin/sh", "-c", user_provided_shell_script,
"sh", /* this is $0 in the script */
torrent_name_str, /* this is $1 in the script */
torrent_category_str, /* this is $2 in the script */
path_str, /* this is $3 in the script */
NUL);