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

2016-11-02 Thread op7ic \x00
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64'
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-unknown-linux-gnu'
-DCONF_VENDOR='unknown' -DLOCALEDIR='/usr/share/locale'
-DPACKAGE='bash' -DSHELL -DHAVE_CONFIG_H   -I.  -I. -I./include
-I./lib  -ggdb3 -O0 -Wno-parenthese$
Machine Type: x86_64-unknown-linux-gnu
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. To replicate this issue use the attached sample below and
execute the following command:

./bash PoC.sh

PoC.sh base64 encoded:

Wy5bLio=

Repeat-By:
echo Wy5bLio= > PoC.sh.b64
base64 -d PoC.sh.b64 > PoC.sh
valgrind ./bash PoC.sh


Vulnerable Code:

475   /* POSIX.2 2.8.3.1.2 says: `An expression containing a `[' that
476  is not preceded by a backslash and is not part of a bracket
477  expression produces undefined results.'  This implementation
478  treats the `[' as just a character to be matched if there is
479  not a closing `]'. */
480   if (c == L('\0'))
481 return ((test == L('[')) ? savep : (CHAR *)0);
482
483   c = *p++;
484   c = FOLD (c);
485
486   if ((flags & FNM_PATHNAME) && c == L('/'))
487 /* [/] can never match when matching a pathname.  */
488 return (CHAR *)0;





ASAN Report (needs to compile with -fsanitize=address):

bash/bash PoC.sh
=
==30562==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x6020d7fb at pc 0x77eb6c bp 0x759126b50b90 sp 0x759126b50b88
READ of size 1 at 0x6020d7fb thread T0
#0 0x77eb6b in brackmatch /opt/bash/lib/glob/sm_loop.c:483
#1 0x77eb6b in gmatch /opt/bash/lib/glob/sm_loop.c:293
#2 0x78c559 in gmatch /opt/bash/lib/glob/sm_loop.c:40
#3 0x78c559 in internal_strmatch /opt/bash/lib/glob/sm_loop.c:42
#4 0x78c559 in xstrmatch /opt/bash/lib/glob/smatch.c:407
#5 0x7750b3 in glob_vector /opt/bash/lib/glob/glob.c:806
#6 0x776fd2 in glob_filename /opt/bash/lib/glob/glob.c:1341
#7 0x63f5a6 in shell_glob_filename /opt/bash/pathexp.c:413
#8 0x60c7a6 in glob_expand_word_list /opt/bash/subst.c:10261
#9 0x60c7a6 in expand_word_list_internal /opt/bash/subst.c:10696
#10 0x4cf780 in execute_simple_command /opt/bash/execute_cmd.c:4153
#11 0x4d9414 in execute_command_internal /opt/bash/execute_cmd.c:802
#12 0x4d9414 in execute_command /opt/bash/execute_cmd.c:405
#13 0x4384d5 in reader_loop /opt/bash/eval.c:180
#14 0x42e5f0 in main /opt/bash/shell.c:792
#15 0x68553d959b44 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
#16 0x4331dc (/opt/bash/bash+0x4331dc)

0x6020d7fb is located 0 bytes to the right of 11-byte region
[0x6020d7f0,0x6020d7fb)
allocated by thread T0 here:
#0 0x68553e16573f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5473f)
#1 0x6e3a05 in xmalloc /opt/bash/xmalloc.c:112

SUMMARY: AddressSanitizer: heap-buffer-overflow
/opt/bash/lib/glob/sm_loop.c:483 brackmatch
Shadow bytes around the buggy address:
  0x0c047fff9aa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ab0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ac0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9af0: fa fa fa fa fa fa fa fa fa fa fd fa fa fa 00[03]
  0x0c047fff9b00: fa fa 00 00 fa fa 06 fa fa fa 00 00 fa fa 00 00
  0x0c047fff9b10: fa fa fd fa fa fa 00 00 fa fa 06 fa fa fa 00 00
  0x0c047fff9b20: fa fa 00 00 fa fa 06 fa fa fa 00 00 fa fa 07 fa
  0x0c047fff9b30: fa fa fd fa fa fa 07 fa fa fa 05 fa fa fa 05 fa
  0x0c047fff9b40: fa fa 02 fa fa fa fd fa fa fa 07 fa fa fa 07 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:   fa
  Heap right redzone:  fb
  Freed heap region:   fd
  Stack left redzone:  f1
  Stack mid redzone:   f2
  Stack right redzone: f3
  Stack partial redzone:   f4
  Stack after return:  f5
  Stack use after scope:   f8
  Global redzone:  f9
  Global init order:   f6
  Poisoned by user:f7
  Contiguous container OOB:fc
  ASan internal:   fe
==30562==ABORTING



Re: 4.4: crash in redir10 test; use after free?

2016-11-02 Thread Chet Ramey
On 11/1/16 12:03 PM, Christian Weisgerber wrote:
> Running the bash 4.4 regression test suite on OpenBSD/amd64, I noticed
> a crash in the redir tests.  Specifically, running redir10.sub with
> bash 4.4 causes it to die with a bus error most of the time.

Thanks for the report.  I can't reproduce this, nor could I reproduce it
on OpenBSD when I tested there before bash-4.4 was released.  (I don't
have ready access to that OpenBSD VM right now, but a use after free should
occur on other systems as well.)

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: out-of-bound read in brackmatch function in "sm_loop.c"

2016-11-02 Thread Chet Ramey
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: 4.4: crash in redir10 test; use after free?

2016-11-02 Thread Christian Weisgerber
Chet Ramey:

> > Running the bash 4.4 regression test suite on OpenBSD/amd64, I noticed
> > a crash in the redir tests.  Specifically, running redir10.sub with
> > bash 4.4 causes it to die with a bus error most of the time.
> 
> Thanks for the report.  I can't reproduce this,

Here's the backtrace:

#0  0x0d78f3634009 in find_pipeline (pid=11813, alive_only=1,   
jobp=0x7f7f61b4) at jobs.c:1481 
#1  0x0d78f36340f5 in find_process (pid=11813, alive_only=1,
jobp=0x7f7f61b4) at jobs.c:1506
#2  0x0d78f3637c53 in waitchld (wpid=-1, block=0) at jobs.c:3531
#3  0x0d78f363795c in sigchld_handler (sig=20) at jobs.c:3411
#4  
#5  0x0d7b431cc78b in ofree (argpool=0xd7be5448350, p=0xd7b4d731360)
at /usr/src/lib/libc/stdlib/malloc.c:1085
#6  0x0d7b431ccc8b in free (ptr=0xd7b0d3aa3e0)
at /usr/src/lib/libc/stdlib/malloc.c:1416
#7  0x0d78f3633a97 in discard_pipeline (chain=0xd7b0d3aa3e0) at jobs.c:1232
#8  0x0d78f364a3c5 in process_substitute (string=0xd7afbf56490 "echo x",
open_for_read_in_child=0) at subst.c:5812

* In process_substitute(), discard_pipeline(last_procsub_child)
  is called.
* discard_pipeline() frees last_procsub_child.
* free() is interrupted by a signal.
* The signal handler eventually calls find_pipeline(), which accesses
  the just-freed memory last_procsub_child points to.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: 4.4: crash in redir10 test; use after free?

2016-11-02 Thread Chet Ramey
On 11/2/16 5:51 PM, Christian Weisgerber wrote:
> Chet Ramey:
> 
>>> Running the bash 4.4 regression test suite on OpenBSD/amd64, I noticed
>>> a crash in the redir tests.  Specifically, running redir10.sub with
>>> bash 4.4 causes it to die with a bus error most of the time.
>>
>> Thanks for the report.  I can't reproduce this,
> 
> Here's the backtrace:

Thanks; that's an easy fix.

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/



shell-expand-line drops quotation marks [FIXED]

2016-11-02 Thread Dabrien 'Dabe' Murphy
[NOTE]  Below is a message I started to write listing a whole slew of 
cases where `shell-expand-line` didn't Do What I Mean.


After familiarizing myself with the source code, however, I was 
pleasantly surprised to discover that there's actually a one line one 
character fix for almost every case I came up with(!)


So, without further ado...

    8<    8<    Cut Here    8<    8<  

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:


### Helper Function ###
prompt% argDump() { while [[ $# -gt 0 ]]; do echo "ARG: '$1'"; 
shift; done; }


prompt% argDump "one two three"
ARG: 'one two three'

whereas `shell-expand-line` does not.

prompt% argDump "one two three" []
prompt% argDump one two three   # FAIL FIXED
ARG: 'one'
ARG: 'two'
ARG: 'three'


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



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

Conversely: [This is one case I have yet to solve; I imagine it involves 
a carefully placed `sh_backslash_quote_for_double_quotes()`, but I 
haven't worked it out, yet...]


prompt% foo='"one two"'# Double quotes inside single quotes
prompt% echo $foo
"one two"

prompt% argDump $foo
ARG: '"one'
ARG: 'two"'# I'm happy with that...
prompt% argDump $foo []
prompt% argDump "one two"
ARG: 'one two' # ARGH!!


Command Injection, anyone?

prompt% bar='; date'
prompt% echo $bar [ ]
Wed Nov  2 15:28:29 EDT 2016# Fool me once...

prompt% echo "$bar" [ ]   # Better Use Protection!
Wed Nov  2 15:28:41 EDT 2016# FAIL FIXED


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:


prompt% echo $'\007'
# Generates ^G (BEL)

prompt% echo $'\007' [  ]
# Becomes...
prompt% echo $\007
prompt% echo $007
prompt% echo -bash07 # WTF??! How'd $0 get in there? FIXED


Ditto history expansion:

prompt% echo $'\007'
prompt% echo !$ []
prompt% echo $\007# FAIL FIXED
$007


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


I thought that maybe prepending "command" or "\" might help, but you'd 
still have:


prompt% alias ls="ls -F"
prompt% alias qq="ls"
prompt% qq /bin/bash
/bin/bash*# with '*', good

prompt% alias qq="\ls"# Note the backslash
prompt% qq /bin/bash
/bin/bash # zero '*', also good

prompt% qq /bin/bash []
prompt% ls /bin/bash
/bin/bash*# WITH '*'... BAD :-( FIXED

(It's worth noting that the shell itself is smart enough to figure out 
when to stop expanding aliases; it seems like that logic would be useful 
here, as well...)



And lastly, even when you ARE doing something "correctly", you *STILL* 
have to watch out for pitfalls:


prompt% alias su='sudo -sEp '\''[sudo] password for %p: '\'''

prompt% su
[sudo] password for dabe: 

prompt% su []
prompt% sudo -sEp [sudo] password for %p:# FAIL FIXED
[sudo]
/bin/bash: password: command not found


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)
{
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!