Shell test or quoting mechanism breaks on parens

[Denys Vlasenko]

On Wed, Jun 4, 2014 at 10:09 PM,  <Dung_Nguyen at dell.com> wrote:
> Dell - Internal Use - Confidential
>
> Thank you very much for your information.
>
> -Dung Nguyen
>
> -----Original Message-----
> From: Joshua Judson Rosen [mailto:jrosen at harvestai.com]
> Sent: Wednesday, June 04, 2014 2:22 PM
> To: Rich Felker
> Cc: Nguyen, Dung; busybox at busybox.net
> Subject: Re: Shell test or quoting mechanism breaks on parens
>
> On 2014-05-19 22:10, Rich Felker wrote:
>> On Mon, May 19, 2014 at 05:02:48PM -0500, Dung_Nguyen at Dell.com wrote:
>>> Dell - Internal Use - Confidential
>>> Hello,
>>> I used to use busybox 1.00 and this command line returns 0 or at least
>>> the output is the same as RHEL 5.3 shell:
>>>
>>> # [ '(' = '(' ] ; rc=$? ; echo $rc ; [ $rc -ne 0 ] && echo EVIL SH
>>> TEST/QUOTE
>>> 0
>>>
>>> However, after I upgraded to busybox 1.20.2 and I see different output:
>>>
>>> # [ '(' = '(' ] ; rc=$? ; echo $rc ; [ $rc -ne 0 ] && echo EVIL SH
>>> TEST/QUOTE
>>> sh: closing paren expected
>>> 2
>>> EVIL SH TEST/QUOTE
>>>
>>> Does anyone know why the output is different with busybox 1.00 or is this
>>> a security issue from cmd line ?
>>
>> As far as I can tell the issue is in the test command (both standalone
>> and in the shell), not in the shell itself. The standard test/[
>> command has very specific rules for how its arguments are interpreted
>> and Busybox does not seem to be honoring them. See:
>>
>> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
>>
>> In particular, if there are 3 arguments and the second is a binary
>> primary (which '=' is) then, regardless of the contents of the first
>> and third, the binary primary is applied to them.
>
> It looks like this broke in commit bd28f6bf7f53ede8df39112d40cb52f2a3d00177
> (Sat Jul 19 08:15:13 2008 +0000), which looks like it was released in
> 1.12.0:
>
> test: fix parser to prefer binop over unop, as coreutils does.
> remove bogus workaround in main(). rename atrocious variables/functions.
> much expand testsuite.
> libbb: fix --help to not affect "test --help"

Indeed.
I was sure general parser will handle it correctly, but ! = !
and ( = ( are nasty, nasty cases!


I fixed this in git.