bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: simple prob?


From: Greg Wooledge
Subject: Re: simple prob?
Date: Tue, 29 Jun 2021 22:57:02 -0400

On Tue, Jun 29, 2021 at 10:11:31PM -0400, Eli Schwartz wrote:
> On 6/29/21 7:29 PM, L A Walsh wrote:
> >>> njobs() { printf ${1:+-v $1} "%s\n" "$(jobs |wc -l)"; }

> 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...

It's not the weirdest thing I've seen.  Without your validation:

unicorn:~$ declare -A jobs
unicorn:~$ njobs 'jobs[first]'

All cool, right?  Uh....

unicorn:~$ touch jobss
unicorn:~$ njobs 'jobs[second]'
unicorn:~$ declare -p jobs
declare -A jobs=([first]=$'0\n' )
unicorn:~$ declare -p jobss
declare -- jobss="0
"

Failure to quote strikes again.  What if we fix it?

unicorn:~$ nqjobs() { printf ${1:+-v "$1"} "%s\n" "$(jobs |wc -l)"; }
unicorn:~$ nqjobs 'jobs[third]'
unicorn:~$ nqjobs 'jobs[sixth]'
unicorn:~$ declare -p jobs
declare -A jobs=([sixth]=$'0\n' [first]=$'0\n' [third]=$'0\n' )

Now it works even if there's a file which matches the glob pattern.

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

Your validation is too strict in this case.  Right idea, wrong
implementation.  Of course, one might argue that allowing an array with
an index is a dangerous precedent, and stick with your strict validation
on principle.  That's a design decision, and I could see either one
being correct depending on the goals.

Oh, and by the way, there's at least one more category of errors that
failure-to-quote is susceptible to:

unicorn:~$ unset foo bar
unicorn:~$ njobs 'foo -v bar'
unicorn:~$ declare -p foo bar
bash: declare: foo: not found
declare -- bar="0
"

Without quotes, we get printf -v foo -v bar '%s\n' "$(jobs|wc -l)"
with the result shown.

With quotes, we get:

unicorn:~$ nqjobs 'foo -v bar'
bash: printf: `foo -v bar': not a valid identifier

It's not robust against code injection attacks (we need actual validation
for that), but at least it gives a sensible error message in some common
cases.

tl;dr: When in doubt, quote it.  There is a REASON we beat this into
people's heads.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]