Re: simple prob?

[Eli Schwartz]

On 6/29/21 7:29 PM, L A Walsh wrote:
> On 2021/06/29 15:49, Greg Wooledge wrote:
>> On Tue, Jun 29, 2021 at 02:58:28PM -0700, L A Walsh wrote:
>>  
>>> njobs() { printf ${1:+-v $1} "%s\n" "$(jobs |wc -l)"; }
>>>
>>> Using that with your input:
>>>
>>> njobs 'x[0$(date >&2)]'
>>>
>>> bash: printf: `x[0$(date': not a valid identifier
>>>     
>>
>> This is because you didn't quote "$1".
> ----
>    $1 should never be quoted -- it is an identifier, and as such
> cannot contain a space.  By quoting it, you are allowing inputs that
> would otherwise be filtered out because they are not valid variable
> names.


You do no filtering right now.

Here's how you filter:

njobs() {
    if [[ $(id -un) = lindawalsh ]]; then
        # Linda agrees to never misuse this function or let
        # anyone else run code on her computer which does
        :
    elif [[ $1 = *[^a-zA-Z0-9_]* ]]; then
        echo "error: invalid characters ${1//[a-zA-Z0-9_]} found in var
${1@Q}" >&2
        echo "hacker detected..." >&2
        # can be exit 13 if used in a script
        return 1
    fi

    printf ${1:+-v $1} "%s\n" "$(jobs |wc -l)";
}


Now, since I am not Linda and I am a big dummy when it comes to code
reuse and I *am* concerned I might stupidly use this code on input
provided by my friend the huge practical joker, I get this:

$ njobs 'x[0$(date >&2)]'
error: invalid characters [$( >&)] found in var 'x[0$(date >&2)]'
hacker detected...

Of course, I don't bother to quote $1 -- I cherish my right to not quote
that variable. But I know it's safe to do so -- if it has word splitting
tokens, they will raise an error.

Greg's fixed example is still caught by my code (unless the code is
running as the Linda login account):

$ njobs 'x[0$(date>&2)]'
error: invalid characters [$(>&)] found in var 'x[0$(date>&2)]'
hacker detected...

It also rejects this perfectly reasonable code, which I arbitrarily
decided not to support because if you want this you're weird:

$ declare -A jobs
$ njobs 'jobs[first run]'
error: invalid characters [ ] found in var 'jobs[first run]'
hacker detected...


Well, the diagnostic jumped to a conclusion that is probably false.


>>   Since you only ever tested
>> the cases where $1 was a valid variable name
> ----
>    It is only designed to work with $1 being an optional, valid variable
> name.
> Anything else should fail.  There are times when putting quotes around a
> var will enable problems.
>> , you never ran into that particular result... until now.
>>   
> ----
>    I never ran into "invalid input" because I didn't program it to
> accept anything other than a variable name there.


You did program it to accept anything other than a variable name there.
You then made a "gentleman's agreement" to not *use* anything other than
a variable name there.

But "use" is a different word than "accept".


>> As you can see, the unquoted $1 underwent word splitting, so you're
>> effectively running printf -v 'x[0$(date' '>&2)]' '%s\n' "...".
>>   
> ----
>    Which is detected as "illegal input" and disallowed.  If you don't
> enable
> some security errors, they can't be as easily introduced.  I assert that
> you
> should never put quotes around something that is supposed to be a
> variable name
> since valid variable names can not be word-split.  Many of the items on
> your
> website about bash cautions, are because bash disallows some sketchy
> constructs.
> That's not a bash-caveat, but a bash feature!


This is a ridiculous argument and you know it. You, personally, are
writing code which does not get used in security contexts, which is your
right. This in no way means that refusing to quote variables which
"cannot be word-split" stops *any* security errors. The "illegal input"
was not related to the security bypass (as Greg points out, removing the
space prevents word splitting and executes the same security bypass code).

Your response should have been:

"""
I assert that you should never put quotes around something that is
supposed to be a variable name since valid variable names can not be
word-split and quoting them implies you would like to handle them as
variable names anyway.

Quotes aren't a security feature. I don't care about security features,
because I don't misuse the code and try to hack my own system. But if
you do want security features, you should detect anything that isn't a
valid variable name and raise an error, then go ahead and use the
variables unquoted.
"""

And hey, I'd even agree with you on that. But I'm also an overly
restrictive person who grumps at people trying to set:

declare -A jobs=(["first run"]=$'0\n' )

Instead you are arguing in bad faith that "That's not a bash-caveat, but
a bash feature!" When strictly speaking your code is flawed, it doesn't
correctly handle indexed arrays with spaces in the key and doesn't
forbid them either.


>> This won't protect against all code injections, of course; only the
>> ones that contain a whitespace character.
>>   
> ----
>    Nothing protect against "all" 'X' or "everything", especially when
> talking
> about security.  Security is best handled as a layered approach with
> different
> layers protecting against different things.  There's no such thing as a
> 'Security Magic Bullet'.  Just because NOTHING protects against "ALL"
> [anything] is
> not a reason to take extreme action. In fact realization that NOTHING is
> perfect
> is one of the better defenses against over-reacting in response to
> supposed security
> flaws.


Removing quotes doesn't protect against anything, it turns a tiny
minority of security flaws into inscrutable errors from printf, makes a
different tiny minority of valid use cases incorrectly error, then fails
to log either problem, abort with errors when it cannot do anything
sensible with the input, or anything else of the sort.


-- 
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User

OpenPGP_signature
Description: OpenPGP digital signature