bug-bash
[Top][All Lists]
Advanced

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

Re: Severe Bash Bug with Arrays


From: Maarten Billemont
Subject: Re: Severe Bash Bug with Arrays
Date: Sat, 5 May 2012 11:04:09 +0200

This mail has gone slightly off-topic.  Hit mark as read if you don't care for 
that sort of thing.

On 27 Apr 2012, at 21:02, Linda Walsh wrote:

> Greg Wooledge wrote:
> 
>> NEVER use eval plus a brace expansion to generate a list.  That's just
>> freakin' evil.  And unsafe:
> 
> But I _like_ evil sometimes!   >:->

I don't care if you like evil in your sexual play or in your vendettas.  When 
you write code, evil is nothing worth liking.  In programming, evil is defined 
as something that is stupid and not thought through. Doing evil has harmful 
side effects. It's not cool to be evil, it's short-sighted.

> But reality is things like that save me time in typing in stuff or writing
> maint scripts on my machine -- they aren't meant for handling user input.
> 
> They take hand editing to gen the paths I want and don't take user input,
> so it's a specialized usage in that case....

It's fine to do short-sighted hacks on the prompt, where you know your input 
well and will notice breakage so that you can recover.

It's not fine to do said things in scripts that run on unpredictable input and 
where you don't always notice breakage until it's caused irreparable harm.  I 
don't really care what lame justifications you come up with to try and justify 
broken system code to yourself, just don't try to convince us with them.

> Maarten Billemont wrote:
> 
>> On 26 Apr 2012, at 01:18, Linda Walsh wrote:
>>> Ishtar:> echo "${b[*]}"
>> Please note that expanding array elements using [*] is usually NOT what 
>> anyone wants.
> 
> It was exactly what I wanted for my example to work.  Please don't suggest
> non-working solutions to the example code I provided.  ${b[@]}  Will NOT
> work as a replacement for [*].  It isn't not BETTER, it is DIFFERENT.
> That's why there are both.

I didn't say never to use "${b[*]}".  I said it's usually not what anyone 
wants.  It's mostly only useful for merging arrays into a single string such as 
for user display of said array.  I explained that.  You are correct, of course, 
there are two syntaxes, they both do something different (when quoted), and 
that's fine.  They each have their use case.

What I *did* say, is never to use ${b[*]}.  And it also so happens that ${b[@]} 
is not different from ${b[*]}.  And I stick with that point.  Never use 
unquoted expansion of arrays, not with [*], not with [@].

>> your array like above, you then follow up by re-splitting the whole thing 
>> using standard wordsplitting, not to mention pathname expanding the 
>> resulting words.
> 
> I followed up?  I don't understand this.

When you leave ${b[*]} unquoted, you expand each element of the array, then ask 
bash to split the whole thing, and then pathname expand the result.  Not only 
did you just throw away any distinction between the different array elements, 
you also made an entirely new distinction (whitespace), and then you're asking 
bash to search the filesystems for any pathnames that match each of the words.  
Seriously, nobody, ever, wants, this.  If they did, they'd have used a string 
instead.  And even then I question the sanity behind it.  Word-splitting is 
useful in a primitive POSIX shell that has no concept like arrays etc. to 
compensate.  In bash, it is the major cause of bugs, and best avoided.

> I would assume they'd have the
> intelligence to know when to use * or @ and I wouldn't have to talk down to 
> them
> and cover basics.

I can't help notice the slight irony in my having to explain to you the 
necessity of quotes with array expansion.

>> You should always recommend the "${b[@]}" variant.
> 
> I shouldn't ALWAYS do anything.  It's dangerous.
> You can do so for your examples... It won't work in the next
> two any more than the last one of the previous...

What?

Anyway, think about this: An array holds elements in a way that each is 
separated from the other.  To put these elements in an array is to want each 
element to be distinct from the others.  Therefore, the default way of 
expanding these elements should preserve that distinction.

Only in rare cases is the actual INTENT of the author to MERGE these elements 
into a single unit, a single string, thus throwing away the distinction between 
the elements.  And in THOSE cases and those cases ALONE should the author use 
"${array[*]}".  So yes, always recommend "${b[@]}", unless you know for a fact 
that the author is looking to merge the elements.

> sort an arg list:
> 
> > args="-dpi -xinerama -clipboard -unixkill -nowinkill -ac +bs -fp 
> > -multiwindow"
> 
> > readarray -t dflts< <( a=( $args ); IFS=$'\n'; echo "${a[*]#?}"|sort -k1.2 )
> 
> > printf "  defaults=(%s)" "${dflts[*]}"
> 
>  defaults=(-ac +bs -clipboard -dpi -fp -multiwindow -nowinkill -unixkill 
> -xinerama)

... What are you trying to prove?  That with a very specific sent of input 
values you can do some ugly and hacky mangling involving arrays and 
wordsplitting to sort a word-list?  Why are you even USING arrays here at all?  
There's no point.  Just to replace spaces with newlines?

Also, you're doing a lot of really bad stuff here.  The point of the array 
structure is to be able to differentiate elements that can hold things like 
spaces and newlines.  Building an array from wordsplitting is senseless: you 
might as well just wordsplit the string wherever you were planning to use the 
array instead.

This snippet completely breaks any arguments that contain spaces and newlines 
in several places.  That is unacceptable.  It works for conveniently chosen 
demo arguments.  But it's still broken code.  Arguments are by definition 
c-strings.  Any code that doesn't treat them as such violates that definition.

Might I point out that you're stripping the leading - and + from your arguments 
and not putting them back?  I don't know where you got the output you claim 
from, but the real output has lost the first character of each word.

> Want to create pathnames using bash's /path/{}/  feature and have the paths
> in an array (for simple paths):
> 
> > a=(lib tmp bin share)
> > (export IFS=,;eval "echo /usr/{${a[*]}}")
> /usr/lib /usr/tmp /usr/bin /usr/share

Don't do this.  You're injecting strings into bash code, and then evaluating 
that code.  You're basically injecting data into code.  Given certain crafted 
filenames, this makes for arbitrary code execution.  I really don't care about 
what kind of justifications you're going to try to come up with for this.

Ignoring the security aspect, this code will break for elements that contain 
commas or braces or quotes or newlines.  And you know what, yes, it's perfectly 
fine for filenames to contain those.  It's not fine for your code to break on 
them.

Grow up, use a loop.  It really doesn't hurt that much.

for dir in "${dirs[@]}"; do
    whatever "/usr/$dir"
done

If you're really a fan of short, there's still options:

whatever "${dirs[@]/#//usr/}"

> But I was pointing out that [*] has its place ... I use [@] alot
> more often than [*], but will use [*]

I agree.

> , though a bit irregularly,
> when I want to get the # of items in an array (where either would work)..
> though I could make a case if it was worth anything to me that #..@ return
> # items and #...[*] return total length of all items...

Which is why I always use {#array[@]} and not ${#array[*]}.  While they do the 
same, the former gives me a warm fuzzy feeling, the latter makes me feel guilty.

> C'est la vi...

vie ;-)

Attachment: smime.p7s
Description: S/MIME cryptographic signature


reply via email to

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