Re: out-of-bound read in brackmatch function in "sm_loop.c"

[op7ic \x00]

awesome - I got couple more like that. Will pipe it over to bug-bash soon.


Cheers,

op7ic

On Wed, Nov 2, 2016 at 6:31 PM, Chet Ramey  wrote:
> On 11/2/16 8:06 AM, op7ic \x00 wrote:
>
>> Bash Version: 4.4
>> Patch Level: 0
>> Release Status: release
>>
>> Description:
>>
>> A out-of-bound read was identified in "brackmatch" function in
>> "sm_loop.c" source file when parsing specially crafted bash source
>> file. The impact is low and will just result in crashing the
>> interpreter.
>
> Thanks for the report.  This will be fixed in the next devel branch
> push, and I will probably release a bash-4.4 for it.
>
> Chet
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>  ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: shell-expand-line drops quotation marks [FIXED]

[Chet Ramey]

On 11/2/16 6:03 PM, Dabrien 'Dabe' Murphy wrote:

> I know this thread
> 
> is a year old, but I do have to say I agree with the OP that
> `shell-expand-line`'s decision to perform Quote Removal seems to violate
> the Principle of Least Astonishment...
> 
> To say `shell-expand-line` "Expand[s] the line as the shell does" seems,
> shall we say, "disingenuous" — if not an outright lie...  The shell
> preserves whitespace:

As does shell-expand-line, but that's not what this example shows.  The
real issue is whether or not shell-expand-line should perform quote
removal.

> Quote Removal makes sense during command EXECUTION (since you wouldn't want
> your quotes passed in with the arguments) but it doesn't make sense during
> (readline) EDITING, IMHO...

OK. So let's talk about a mechanism to provide alternate behavior.  The
conventional way to do that in the context of key bindings is to modify
behavior based on whether or not the user supplies a numeric argument
(e.g., M-1M-C-e).
> 
>  
> Consider the following variable expansion:
> 
> prompt% foo="one two"
> prompt% argDump $foo
> ARG: 'one'
> ARG: 'two'
> 
> prompt% argDump "$foo"
> ARG: 'one two'
> 
> prompt% argDump $foo []
> prompt% argDump one two# So far, so good, actually...
> 
> prompt% argDump "$foo" []
> prompt% argDump one two# FAIL FIXED

Sure, you're just restating your argument several times.

> At the very least, I would expect `shell-expand-line` to be more or less
> idempotent; expanding the line multiple times shouldn't change the behavior
> of the command that actually gets executed:

This is not a reasonable expection.  Think of it as running a command
through `eval' several times.


> I understand it's hard to do the Right Thing sometimes:  [Still unsolved]
> 
> prompt% alias ls="ls -F"
> prompt% ls []
> prompt% ls -F []
> prompt% ls -F -F []
> prompt% ls -F -F -F

This is actually the correct behavior.  `ls' has an alias, and that alias
is expanded.

> 
> So what's the fix, you might ask?
> diff --git a/bashline.c b/bashline.c
> index 238a190..e17a49d 100644
> --- a/bashline.c
> +++ b/bashline.c
> @@ -2689,7 +2689,7 @@ shell_expand_line (count, ignore)
>/* If there is variable expansion to perform, do that as a separate
>  operation to be undone. */
>new_line = savestring (rl_line_buffer);
> -  expanded_string = expand_string (new_line, 0);
> +  expanded_string = expand_string (new_line, 1);
>FREE (new_line);
>if (expanded_string == 0)
> {

Nice catch.  This is pretty close, and gets you most of the way you want
to go. You'd also like a way to inhibit process and command substitution
during shell-expand-line and allow those to be performed as part of
execution to avoid side effects.

> If you're really concerned that people are actually relying on the old
> behavior, I'm sure it would be easy to create some sort of
> "shell-expand-preserve-quotes" readline variable, or some such...  Show me
> where to submit a Pull Request and I'd be happy to whip one up!  :-D

The old behavior will remain the default for now -- it's been this way for
about 25 years, after all -- but the inhibiting-quote-removal behavior
(and probably command and process substitution as well) will be available
if the user supplies a numeric argument.

> PS — Another example where `shell-expand-line` decidedly does NOT "expand
> the line as the shell does" is with globs and tilde prefixes, but I'm aware
> this is a known limitation:
> 
> prompt% echo ~/.bash* []

It's true: globbing is not one of the shell word expansions that this
function performs, or has ever performed.  There's a separate key binding
that does this.

Tilde expansion is a little trickier, since shell-expand-line does not run
the line buffer through the shell parser to break it into words and tilde
expansion only happens at the start of a word.  (If you were to start the
command with an unquoted tilde, you'd find tilde expansion is performed.)

That's one of the reasons there is a separate key binding to perform tilde
expansion on the current word.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Severe IFS glob bug needs patch [was: Setting IFS='*' causes bash to freeze on tab completion]

[Martijn Dekker]

Op 02-07-16 om 11:30 schreef Andreas Schwab:
> "Ibrahim M. Ghazal"  writes:
> 
>> After further investigation, the problem seems to be that when IFS is
>> set to '*', case *) doesn't get executed. Is this the intended
>> behavior?
>>
>> For example:
>>
>> IFS='*'; case "foo" in *) echo "got here";; esac
>>
>> doesn't print "got here".
> 
> The same issue exists for the other glob special characters ? [ ] .

Op 02-07-16 om 15:37 schreef Chet Ramey:
> Yep, that's a bug.  Thanks for the report and reproducer.  The problem is
> that the `*' ends up being matched literally instead of as a matching
> character.  This will be fixed in the next release of bash.

Unfortunately, this was only partially fixed in 4.4. The glob pattern
matching in 'case' and '[[' was fixed, but pathname expansion is still
broken:

$ IFS='?*[]'
$ ls *
ls: *: No such file or directory
$ touch a && ls ? [a]
ls: ?: No such file or directory
ls: [a]: No such file or directory

(Needless to say, this works fine on every non-bash shell.)

While this bug is relatively rarely triggered (since glob characters are
rarely put in IFS), it's actually REALLY SEVERE in its effects. Many if
not most scripts use 'case'/'[[' glob pattern matching or pathname
expansion at some point, so breaking either of these under any
circumstances is a big deal.

And when things do break, they break catastrophically. Not just your own
program breaks, but any third-party add-ons or libraries you're using
break as well.

While including any of the characters *, ?, [, ] in IFS might be rare,
strings do sometimes have values separated by one of these, so it's a
perfectly legitimate use case to split something on these characters.

So, in addition to fixing pathname expansion for 4.4, I hope you would
consider releasing a patch fixing both aspects of this bug for earlier
versions, at least down to 3.2 (which will forever be the default
version on Mac OS X, since Apple won't touch the GPL3).

Thanks,

- Martijn

Re: shell-expand-line drops quotation marks [FIXED]

[Dabrien 'Dabe' Murphy]

On 11/3/16, 4:21 PM, Chet Ramey wrote:

Quote Removal makes sense during command EXECUTION (since you wouldn't want
your quotes passed in with the arguments) but it doesn't make sense during
(readline) EDITING, IMHO...


OK. So let's talk about a mechanism to provide alternate behavior.  The
conventional way to do that in the context of key bindings is to modify
behavior based on whether or not the user supplies a numeric argument
(e.g., M-1M-C-e).


As if Esc + Control-E wasn't already enough of a contortion!  «grin»

I've actually bound `shell-expand-line` to ^X+Tab --- sort of an über 
expansion, if you will...  I guess one could always add Yet Another 
level of indirection:


prompt% bind '"\C-x\C-i":"\e1\e\C-e"'   # Ow, my brain!



At the very least, I would expect `shell-expand-line` to be more or less
idempotent; expanding the line multiple times shouldn't change the behavior
of the command that actually gets executed:


This is not a reasonable expection.  Think of it as running a command
through `eval' several times.


prompt% echo $'\007'


prompt% eval eval eval eval eval echo $'\007'


(Okay, so I know you were thinking more along the lines of `eval $(eval 
$(eval echo echo echo 'Hi There!') )` but I don't think anybody would 
expect that to be repeatable...)


I will have to think a little bit more, however, about what I mean when 
I do:


prompt% foo="one two"   # two words
prompt% bar='$foo'  # bare reference
prompt% baz='"$foo"'# double-quoted reference
prompt% bad=\'\$foo\'   # single-quoted reference

Currently I don't like *EITHER* scenario:

old% echo $bad   []
old% echo '$foo' []
old% echo $foo   []
old% echo one two

new% echo $bad   []
new% echo '$foo' []
new% echo 'one two'

In my mind, '$foo' would be unexpandable...  Ditto:

prompt% echo \$foo   # Should not change anything...


 prompt% alias ls="ls -F"
 prompt% ls []
 prompt% ls -F []
 prompt% ls -F -F []
 prompt% ls -F -F -F


This is actually the correct behavior.  `ls' has an alias, and that alias
is expanded.


I don't disagree that that's what it's doing, but it seems like it could 
be ensmartened, somehow, to recognize when an alias would expand to 
itself...


I'm also on the fence about whether I would prefer:

prompt% alias zz=yy
prompt% alias yy=xx
prompt% alias xx=ls

prompt% zz []
prompt% yy

or:

prompt% zz []
prompt% ls

THAT'S the kind of thing I'd expect a numeric prefix argument to 
control: whether to recurse or not...  And the logic already exists to 
prevent the following circular definition from getting stuck in an 
infinite loop; I would imagine it could be reused to short-circuit the 
expansion, as well:


prompt% alias xx=yy
prompt% alias yy=zz
prompt% alias zz=xx

prompt% xx
-bash: xx: command not found


Nice catch.  This is pretty close, and gets you most of the way you want
to go. You'd also like a way to inhibit process and command substitution
during shell-expand-line and allow those to be performed as part of
execution to avoid side effects.


Process substitution, yeah, I can't actually imagine it ever being 
useful to actually SEE the "/dev/fd/##" --- any my fix actually inhibits 
it already...  Though after you mentioned it, I was actually kind of 
surprised to see the old behavior even worked at all:


old% cat <(echo Hi There) []
old% cat /dev/fd/63   []
Hi There

COMMAND substitution, however...  I could see that being useful:

prompt% echo server-$(date +%Y%m%d).log  []
prompt% echo server-20161103.log

Or here's a more practical example:

prompt% vi $(grep -ri -l old_text lib/)
prompt% git add !*    # WHOOPS! "old_text" doesn't match anymore

prompt% vi $(grep -ri -l old_text lib/) []
prompt% vi lib/Module/foo.c lib/Module/foo.h lib/main.c
prompt% gid add !*# OKAY


As an aside, I did not expect this:

prompt% echo server-$(date +%Y%m%d).log
server-20161103.log

prompt% echo !* []
prompt% echo server-$ ( date +%Y%m%d ) .log
-bash: syntax error near unexpected token `('

But that's not actually related to shell-expand-line, itself:

prompt% echo server-$(date +%Y%m%d).log
server-20161103.log

prompt% echo !*
-bash: syntax error near unexpected token `('

### Quotes, however, seem to make things work again...

prompt% echo "server-$(date +%Y%m%d).log"
server-20161103.log

prompt% echo !*
server-20161103.log

prompt% echo !* []
prompt% echo "server-20161103.log"   # OKAY


The old behavior will remain the default for now -- it's been this way for
about 25 years, after all -- but the inhibiting-quote-removal behavior
(and probably command and process substitution as well) will be available
if the user supplies a nu

Re: Severe IFS glob bug needs patch [was: Setting IFS='*' causes bash to freeze on tab completion]

[Eduardo Bustamante]

I agree with everything, except calling it severe. This is
self-inflicted harm, and easy to work around

devel branch fails to build when using bash malloc due to missing errno.h include

[Eduardo A . Bustamante López]

I'm running configure with the following flags:

|./configure CC=gcc-6

The error I get while building:

|make[1]: Entering directory '/home/dualbus/local/src/gnu/bash/lib/malloc'
|gcc-6  -I. -I../.. -I../.. -I../../include -I../../lib  -DHAVE_CONFIG_H 
-DSHELL  -Wall -g -ggdb -O0 -Wno-parentheses -Wno-format-security -DRCHECK 
-Dbotch=programming_error -DMALLOC_DEBUG  -c malloc.c
|malloc.c: In function ‘xsplit’:
|malloc.c:472:21: warning: unused variable ‘split_max’ [-Wunused-variable]
|   int nbuck, nblks, split_max;
| ^
|malloc.c: In function ‘posix_memalign’:
|malloc.c:1143:12: error: ‘EINVAL’ undeclared (first use in this function)
| return EINVAL;
|^~
|malloc.c:1143:12: note: each undeclared identifier is reported only once 
for each function it appears in
|malloc.c:1153:10: error: ‘ENOMEM’ undeclared (first use in this function)
|   return ENOMEM;
|  ^~
|malloc.c:1154:1: warning: control reaches end of non-void function 
[-Wreturn-type]
| }
| ^
|At top level:
|malloc.c:669:1: warning: ‘malloc_debug_dummy’ defined but not used 
[-Wunused-function]
| malloc_debug_dummy ()
| ^~
|Makefile:62: recipe for target 'malloc.o' failed
|make[1]: *** [malloc.o] Error 1
|make[1]: Leaving directory '/home/dualbus/local/src/gnu/bash/lib/malloc'
|Makefile:399: recipe for target 'lib/malloc/libmalloc.a' failed
|make: *** [lib/malloc/libmalloc.a] Error 1

This is fixed by adding an errno.h include inside lib/malloc/malloc.c

-- 
Eduardo Bustamante
https://dualbus.me/

memory leak in execute_simple_command when dofork is true

[Eduardo A . Bustamante López]

I ran the configure script with the following arguments:

hp% ./configure CC=gcc-6 CFLAGS='-Wall -g -ggdb -O0 -fsanitize=address' 
LDFLAGS=-fsanitize=address --without-bash-malloc

Which enables the LeakSanitizer 
(https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer).

It detected the following memory leak:

hp% ./bash -c ': & wait'

=
==5784==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x7f0ec8737d28 in malloc 
(/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
#1 0x559e5096b6cb in xmalloc 
/home/dualbus/local/src/gnu/bash/xmalloc.c:112
#2 0x559e508aa246 in execute_simple_command 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:4105
#3 0x559e50899914 in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:802
#4 0x559e508a19a9 in execute_connection 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:2581
#5 0x559e5089a6e9 in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:971
#6 0x559e5097c900 in parse_and_execute 
/home/dualbus/local/src/gnu/bash/builtins/evalstring.c:443
#7 0x559e5086597f in run_one_command 
/home/dualbus/local/src/gnu/bash/shell.c:1399
#8 0x559e50863eaa in main /home/dualbus/local/src/gnu/bash/shell.c:724
#9 0x7f0ec7eca2b0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: 2 byte(s) leaked in 1 allocation(s).

The leak is in line 4105, since the savestring function is called (which just a
strcpy/malloc wrapper), but the allocated memory is not FREE'd.

hp% cat -n execute_cmd.c|sed -n '4100,4110p'
  4100   vast majority of cases. */
  4101maybe_make_export_env ();
  4102
  4103/* Don't let a DEBUG trap overwrite the command string to be 
saved with
  4104   the process/job associated with this child. */
  4105if (make_child (savestring (the_printed_command_except_trap), 
async) == 0)
  4106  {
  4107already_forked = 1;
  4108simple_command->flags |= CMD_NO_FORK;
  4109
  4110subshell_environment = SUBSHELL_FORK; /* XXX */

The issue is more evident when running long commands:

# This will start chewing chunks of ~2MB
hp% bash -c 'while :; do : "$(printf '%.sx' {1..2097152})" & wait; done'

I think the fix is easy, since we just have to store the result of savestring
in a temporary pointer, so that we can later call FREE(p).

-- 
Eduardo Bustamante
https://dualbus.me/

Re: memory leak in execute_simple_command when dofork is true

[Eduardo A . Bustamante López]

Actually, there are more leaks in execute_cmd.c, I'm just going to list them 
here.

---
hp% ./bash -c '(:)'

=
==7854==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 6 byte(s) in 1 object(s) allocated from:
#0 0x7fb0c2580d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
#1 0x562cb665d6cb in xmalloc /home/dualbus/local/src/gnu/bash/xmalloc.c:112
#2 0x562cb658ab1c in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:610
#3 0x562cb666e900 in parse_and_execute 
/home/dualbus/local/src/gnu/bash/builtins/evalstring.c:443
#4 0x562cb655797f in run_one_command 
/home/dualbus/local/src/gnu/bash/shell.c:1399
#5 0x562cb6555eaa in main /home/dualbus/local/src/gnu/bash/shell.c:724
#6 0x7fb0c1d132b0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: 6 byte(s) leaked in 1 allocation(s).

---
hp% ./bash -c 'coproc :; sleep 1'

=
==8716==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7fc17740ad28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
#1 0x55d4b48826cb in xmalloc /home/dualbus/local/src/gnu/bash/xmalloc.c:112
#2 0x55d4b47b72c6 in execute_coproc 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:2301
#3 0x55d4b47af935 in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:593
#4 0x55d4b47af0fb in execute_command 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:405
#5 0x55d4b47b8f54 in execute_connection 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:2609
#6 0x55d4b47b16e9 in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:971
#7 0x55d4b4893900 in parse_and_execute 
/home/dualbus/local/src/gnu/bash/builtins/evalstring.c:443
#8 0x55d4b477c97f in run_one_command 
/home/dualbus/local/src/gnu/bash/shell.c:1399
#9 0x55d4b477aeaa in main /home/dualbus/local/src/gnu/bash/shell.c:724
#10 0x7fc176b9d2b0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).

---
hp% ./bash -c '::; :'   
./bash: ::: command not found

=
==10176==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 3 byte(s) in 1 object(s) allocated from:
#0 0x7f003afe5d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
#1 0x561a310216cb in xmalloc /home/dualbus/local/src/gnu/bash/xmalloc.c:112
#2 0x561a30f641b3 in execute_disk_command 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:5194
#3 0x561a30f61b01 in execute_simple_command 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:4403
#4 0x561a30f4f914 in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:802
#5 0x561a30f4e0fb in execute_command 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:405
#6 0x561a30f57f54 in execute_connection 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:2609
#7 0x561a30f506e9 in execute_command_internal 
/home/dualbus/local/src/gnu/bash/execute_cmd.c:971
#8 0x561a31032900 in parse_and_execute 
/home/dualbus/local/src/gnu/bash/builtins/evalstring.c:443
#9 0x561a30f1b97f in run_one_command 
/home/dualbus/local/src/gnu/bash/shell.c:1399
#10 0x561a30f19eaa in main /home/dualbus/local/src/gnu/bash/shell.c:724
#11 0x7f003a7782b0 in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: 3 byte(s) leaked in 1 allocation(s).

-- 
Eduardo Bustamante
https://dualbus.me/