views:

62

answers:

2

I have a bash script that I am modifying to accept key=value pairs from stdin. (It is spawned by xinetd.) How can I safely convert those key=value pairs into environment variables for subprocesses?

I plan to only allow keys that begin with a predefined prefix "CMK_", to avoid IFS or any other "dangerous" variable getting set. But the simplistic approach

function import ()
{
    local IFS="="
    while read key val; do
        case "$key" in CMK_*)
            eval "$key=$val";;
        esac
    done
 }

is horribly insecure because $val could contain all sorts of nasty stuff. This seems like it would work:

shopt -s extglob
function import ()
{
    NORMAL_IFS="$IFS"
    local IFS="="
    while read key val; do
        case "$key" in CMK_*([a-zA-Z_]) )
            IFS="$NORMAL_IFS"
            eval $key='$val'
            export $key
            IFS="="
            ;;
        esac
    done
 }

but (1) it uses the funky extglob thing that I've never used before, and (2) it's complicated enough that I can't be comfortable that it's secure.

My goal, to be specific, is to allow key=value settings to pass through the bash script into the environment of called processes. It is up to the subprocesses to deal with potentially hostile values getting set.

I am modifying someone else's script, so I don't want to just convert it to Perl and be done with it. I would also rather not change it around to invoke the subprocesses differently, something like

#!/bin/sh
...start of script...
perl -nle '($k,$v)=split(/=/,$_,2); $ENV{$k}=$v if $k =~ /^CMK_/; END { exec("subprocess") }'
...end of script...

Update: What I ended up using for the key check is:

if [ "$key" = "${key%[^a-zA-Z_0-9]*}" ]; then

It doesn't require extglob (global setting) or regexes (only in bash >= 3). It works by throwing out anything not in a whitelist of allowed characters, then comparing the result to the original. If nothing was thrown out, then the whole key must contain only whitelisted characters.

A: 

one way is know what kind of nasty stuff you are going to get. then sanitize the value as your function reads from stdin. eg (bash >3.2)

function import ()
{
    local IFS="="
    while read key val; do
        case "$key" in CMK_*)
            # you can use case/esac here if you like
            if [[ $val =~ "some bad stuff" ]] ;then
                 echo "bad stuff found" 
                 echo "decide to exit or not here"
            else
                 eval "$key=$val";;
            fi

        esac
    done
 }
ghostdog74
That makes me nervous. I'd need to predict all possible forms of nasty stuff. I would rather rely on a whitelist than a blacklist.
sfink
A: 

It's much safer to use declare than eval:

shopt -s extglob
function import ()
{
    NORMAL_IFS="$IFS"
    local IFS="="
    while read key val; do
        case "$key" in 
            CMK_*([a-zA-Z_]) )
                IFS="$NORMAL_IFS"
                declare $key="$val"  2>/dev/null || echo "Bad key"
                IFS="="   # why set this here?
                ;;
            *)
                echo "Bad key"
                ;;
        esac
    done
}

If you don't want to use extglob, you can use a regex matching test:

while ...
if [[ $key =~ CMK_ ]]    # or something like: [[ $key =~ CMK_[[:alpha:]] ]]
then
    declare ...
else
    echo "Bad key"
fi

Also, see this.

Dennis Williamson
re: why I was re-setting IFS. In some conditions, I was getting an error "CMK_TEST: command not found" because it was splitting on the = sign. I'm still a bit confused as to when that was happening, because in simple tests it doesn't.
sfink
I didn't know about declare. That's a useful bit. Avoiding eval is always good. But I've ended up using something that avoids using either extglob or regexes (thus satisfying my secret agenda of staying within a portable subset of bash delineated entirely by my arbitrary experiences and comfort level.) I'll update my main post with what I ended up using.
sfink