5
\$\begingroup\$

I have built a shell function aimed to perform the following:

  • Given a string as first argument, perform safe expansions (i.e. those that cannot cause arbitrary code execution and only produce string output)

  • Save the output to a variable the name of which is provided as second argument, and can be considered safe for the purpose of this question

The code is as follows :

#
# Argument 1 : string to safely expand
# Argument 2 : name of the variable to which expanded value will be assigned
#
safeval()
{
local _v=$1
_v=${_v//\$\(/\$$'\1'}
_v=${_v//\$\[/\$$'\2'}
_v=${_v//'`'/$'\3'}
_v=${_v//\\ /$'\4'}
_v=${_v// /$'\5'}
eval printf -v _v %b "$_v" || return 1
_v=${_v//$'\1'/\(}
_v=${_v//$'\2'/\[}
_v=${_v//$'\3'/'`'}
_v=${_v//$'\4'/\\ }
_v=${_v//$'\5'/ }
printf -v "$2" %s "$_v"
}

This code aims to achieve its purpose by doing the following :

  • Replace all occurrences of $(, $[ and ` with (hopefully) safe strings.
  • Then perform eval (the remaining expansions are performed)
  • Then replace the other way around

This means that expansions that are not performed will be kept as is. There may be cases where nested expansions have the outer expansion execute, but the inner expansion be blocked, which maybe could create incorrect output or failure; as long as it just fails without doing anything nasty from a security standpoint, this is OK for me, as I am going for safety first.

Here are example cases (function safeval assumed to be already loaded):

A=1
safeval '$A' B
echo "$B"   # Echoes "1" (without the double quotes)
safeval '$A$(ls /)$A' C
echo "$C"   # Echoes "1$(ls /)1" (without the double quotes)

Any feedback about the safety (or lack thereof) and potential failure modes that I have missed is especially welcome.

This function is intended to allow the use of values read from configuration files in which (safe) expansions and shell-type quoting would be allowed.

EDIT

I found a bug in handling of spaces and double quotes. On the eval printf line, variable _v is expanded, and if it contains double quotes and spaces, printf sees many arguments (spaces are stripped due to word splitting). To fixe this the additional code :

  • Replace already-escaped spaces with a non-printable character
  • Escape remaining spaces so that printf will see only one argument
  • At the end, put the escaped spaces back in
\$\endgroup\$
  • \$\begingroup\$ Why do you think you need a safe eval? Your examples look rather hypothetical. \$\endgroup\$ – 200_success Feb 20 '17 at 18:36
  • 1
    \$\begingroup\$ I am implementing config file handling. This is currently done with a simple "eval" on config files the lines of which contain (admin controlled) assignments, but I need to make that work for user-controlled config files too. I might not need expansions that badly (though they are nice to have as a generic mechanism), but I need eval to do proper quote evaluation (or else I will need to write my own parsing code). So you could say that having to handle quotes correctly forces me to have a safe version of eval, and not blocking safe expansions is a bonus feature at no additional cost. \$\endgroup\$ – Fred Feb 20 '17 at 18:47
  • \$\begingroup\$ One note : this function will perform globs as per the currently active shells. This may be useful, so I am not adding code to prevent it, but it can cause unexpected (while not incorrect, I think) results in some cases. Globbing should be disabled if this behavior is problematic. \$\endgroup\$ – Fred Feb 25 '17 at 21:16
1
\$\begingroup\$

The way you implemented escaping some symbols, running eval, and finally restoring the escaped symbols, looks fine.

Have you escaped everything to make this safe? That's hard to tell. And will it be future-proof? Now that's impossible to tell. You just cannot know if a future version of Bash will introduce something that should be escaped and leave your script vulnerable until patched. That's unlikely for sure, but this is an ugliness that any blacklisting approach cannot escape.

Most importantly, do you really need this? (As @200_success pointed out in a comment.) It seems you don't. If you need to parse key=value pairs from a config file, then I suggest to do just that, implement just enough parsing that you need. The solution will be specialized for the specific use case, but that will make it safe, and free of any present or future problems with eval.

\$\endgroup\$
  • 1
    \$\begingroup\$ I need to read from config files that may contain variables to expand (I do plenty of parsing on static strings, this is of course best whenever it does the job, but this is not one of those cases). I also want to be able to use shell-style quoting. But I do not want to write a parser for all of that, lazy me. You do have good points, which at this time are not going to convince me to avoid doing this. Given the small scope of use for this hack, trying to make it safe is almost an exercise in style, and an occasion to learn a thing or two in the process. Thanks for your help! \$\endgroup\$ – Fred Sep 15 '17 at 19:29
0
\$\begingroup\$

I strongly advise against going down this path. There's an article I've read but can't find now about how to do secure shell programming, in which the author goes to great length to demonstrate that it can be done (for one example), but it's so convoluted and involved it's just not worth it.

I don't know your exact use case, but in the past I've implemented a shell function that checks if a user config file (pretty much just shell assignments) is "sanitized" or not that goes as follows:

function sanitized_source
{
    typeset unsanitized_count=0
    while (( $# > 0 ))
    do
        typeset source=$1
        if [[ -f $source && -r $source ]]
        then
            &>/dev/null command -p env -i PATH='/' /bin/bash -r "$source" || (( ++unsanitized_count ))
        else
            printc -warning "$source is not a readable file"
        fi
        shift
    done
    return ${unsanitized_count}
}

And to use it:

if sanitized_source "${project_config_file:?}"
then
    source "${project_config_file}"
else
    return $?
fi

The use of restricted shell and the modification of PATH to a directory where only root has write access and where no external programs are expected to exist should be able to limit the damage that can be done. I think this is a nice trade-off between security and simplicity.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.