double-free in bashline.c

2023-03-16 Thread Grisha Levit
A few functions in bashline.c free static variables but do not assign
to them until after calling bash_tilde_expand, which may
throw_to_top_level.  If SIGINT is received at an inopportune time,
these variables may be free-d again.

diff --git a/bashline.c b/bashline.c
index 2745c4dd..b5c0a49f 100644
--- a/bashline.c
+++ b/bashline.c
@@ -1970,6 +1970,7 @@ command_word_completion_function (const char
*hint_text, int state)
free (dequoted_hint);
   if (hint)
free (hint);
+  dequoted_hint = hint = (char *)NULL;

   mapping_over = searching_path = 0;
   hint_is_dir = CMD_IS_DIR (hint_text);
@@ -2252,6 +2253,7 @@ globword:
free (fnhint);
   if (filename_hint)
free (filename_hint);
+  fnhint = filename_hint = (char *)NULL;

   filename_hint = sh_makepath (current_path, hint, 0);
   /* Need a quoted version (though it doesn't matter much in most
@@ -2397,7 +2399,10 @@ command_subst_completion_function (const char
*text, int state)
   start_len = text - orig_start;
   filename_text = savestring (text);
   if (matches)
-   free (matches);
+   {
+ free (matches);
+ matches = (char **)NULL;
+   }

   /*
* At this point we can entertain the idea of re-parsing
@@ -3873,9 +3878,11 @@ glob_complete_word (const char *text, int state)
 {
   rl_filename_completion_desired = 1;
   FREE (matches);
+  matches = (char **)NULL;
   if (globorig != globtext)
FREE (globorig);
   FREE (globtext);
+  globorig = globtext = (char *)NULL;

   ttext = bash_tilde_expand (text, 0);



Re: command_not_found_handle not run when PATH is empty

2023-03-16 Thread Chet Ramey

On 3/8/23 1:33 PM, Moshe Looks wrote:


Bash Version: 5.1
Patch Level: 16
Release Status: release

Description:
When PATH is empty, the command_not_found_handle function is not
called when a command is not found.


I agree that bash should do a path search in the current directory when
PATH is the empty string, so I'll revert the change from 1994 and do that.

When PATH is unset, there's no path search, and no `not finding' the
command -- you just try to exec something in the current directory (or
you don't -- behavior varies among shells).

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




Re: command_not_found_handle not run when PATH is empty

2023-03-16 Thread Chet Ramey

On 3/9/23 5:42 PM, Moshe Looks wrote:

Thank you this is very illuminating and makes it clear that my naive
one-line fix would be inappropriate. Given the current state of
affairs there is really no good reason for PATH to ever be unset or
set to empty vs. explicitly set to '.', right? So might be worth
spelling out in the docs that this is inadvisable :).


There shouldn't be any difference between PATH=. and PATH=, but why not
use the former and make your intent clear?

Behavior varies with `unset PATH', so it makes sense not to use it. The
Bourne shell didn't even allow PATH to be unset.

Chet

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




bash login shell detection broken using default compile options

2023-03-16 Thread Tycho Kirchner

Hi,
when compiling bash with a plain

mkdir build; cd build; ../configure && make

the login shell detection (leading dash: -bash) is apparently broken - at 
least, ~/.bashrc is not sourced.
This happens in all tested versions, from 4.4 until the latest release 5.2.15.
Interestingly, using distribution provided binaries, everything is fine (tested 
on Debian Bullseye, Debian testing and OpenSuse Leap).

Steps to reproduce:

Compile bash from source using default compile options (see above).

Put

echo hi from bashrc >&2

to the beginning of your ~/.bashrc.

Create the following python script and make it executable:

~/test.py
__
#!/usr/bin/env python3
import os
import sys
bash_path=sys.argv[1]
os.execv(bash_path, ['-bash', 'foobar',])
__

Run the script with the distribution provided binary:

$ ~/test.py /bin/bash
hi from bashrc
-bash: foobar: No such file or directory

Run the script with your own compiled binary:

$ ~/test.py ~/tmp/bash/bash-5.2.15/build/bash
-bash: foobar: No such file or directory

--> ~/.bashrc was apparently not sourced.

Thanks
Tycho




Re: bash login shell detection broken using default compile options

2023-03-16 Thread Greg Wooledge
On Thu, Mar 16, 2023 at 05:21:23PM +0100, Tycho Kirchner wrote:
> Hi,
> when compiling bash with a plain
> 
> mkdir build; cd build; ../configure && make
> 
> the login shell detection (leading dash: -bash) is apparently broken - at 
> least, ~/.bashrc is not sourced.

What you're referring to is an optional compile-time feature, which
is not enabled in the vanilla bash source code.

> This happens in all tested versions, from 4.4 until the latest release 5.2.15.
> Interestingly, using distribution provided binaries, everything is fine 
> (tested on Debian Bullseye, Debian testing and OpenSuse Leap).

Those particular Linux vendors must have chosen to enable it.

/* Define this to make non-interactive shells begun with argv[0][0] == '-'
   run the startup files when not in posix mode. */
/* #define NON_INTERACTIVE_LOGIN_SHELLS */

If you want to turn this on, it's in config-top.h.

> echo hi from bashrc >&2
> 
> to the beginning of your ~/.bashrc.

For the record, ~/.bashrc is not a file that's read (directly) by bash
when invoked in this mode.  You must be sourcing it from your ~/.profile
or ~/.bash_{profile,login} file.

Also for the record, you can skip Python in your reproducer.  Bash'e
exec -a can do the same thing:

unicorn:~$ (exec -a -bash /bin/bash nosuchscript)
hi from .profile
-bash: nosuchscript: No such file or directory
unicorn:~$ (exec -a -bash /usr/local/bin/bash-5.2 nosuchscript)
-bash: nosuchscript: No such file or directory

(I'm on Debian too, so I can confirm that Debian has enabled this
NON_INTERACTIVE_LOGIN_SHELLS feature in its /bin/bash.)



Re: bash login shell detection broken using default compile options

2023-03-16 Thread Martin Schulte
Hello Tycho!

> the login shell detection (leading dash: -bash) is apparently broken - at 
> least, ~/.bashrc is not sourced.

As far as I understand the manual ~/.bashrc is not sourced from a login-shell, 
see https://www.gnu.org/software/bash/manual/bash.html#Bash-Startup-Files

At least in Debian 11 the default ~/.profile cares for sourcing ~/.bashrc, that 
might explain the effect you describe.

Best regards,

Martin



rl_filename_quoting_function restoration

2023-03-16 Thread Grisha Levit
bash_glob_complete_word modifies rl_filename_quoting_function, which
can fail to be restored if bash_tilde_expand handles a SIGINT while
glob-complete-word is running

Handling the restore in bashline_reset() seems to solve the issue:

diff --git a/bashline.c b/bashline.c
index 2745c4dd..7c3812eb 100644
--- a/bashline.c
+++ b/bashline.c
@@ -685,6 +685,7 @@ bashline_reset (void)
   complete_fullquote = 1;
   rl_filename_quote_characters = default_filename_quote_characters;
   set_filename_bstab (rl_filename_quote_characters);
+  rl_filename_quoting_function = bash_quote_filename;

   set_directory_hook ();
   rl_filename_stat_hook = bash_filename_stat_hook;



Re: The memory occupied by bash has been increasing due to the fork bomb

2023-03-16 Thread Chet Ramey

On 3/11/23 6:41 AM, zju wrote:


The change was made in this pr:
https://git.savannah.gnu.org/cgit/bash.git/commit/?h=devel&id=ea31c00845c858098d232bd014bf27b5a63a668b
 



The logic goes like this for interactive shells (non-interactive shells
don't ignore SIGTERM):


block SIGTERM in the parent
set the SIGTERM handler to SIG_DFL in the parent
fork, so the child, which still has SIGTERM blocked, has no pending signals
in the parent, restore the SIGTERM handler to SIG_IGN
if fork fails, abandon, reset the signal mask, and jump back to top level
- this is where other pipeline processes get sent SIGTERM   
in the child, unblock SIGTERM -- if it received a SIGTERM after fork, it
will handle it now and exit due to the disposition being SIG_DFL

You might want to run a system call tracer and look at calls to kill and
sigaction.

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




Re: The memory occupied by bash has been increasing due to the fork bomb

2023-03-16 Thread Chet Ramey

On 3/10/23 9:50 PM, zju wrote:


So is it possible to optimize the continuous growth of memory occupied by child 
processes?


You'll have to show that it's an actual leak, not just memory bash is
allocating and using for its own purposes. Maybe a more controlled test
instead of a fork bomb that you can run valgrind on.

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




Re: global-buffer-overflow in parse.y

2023-03-16 Thread Grisha Levit
On Mon, Mar 6, 2023 at 9:16 AM Chet Ramey  wrote:
> Thanks for the report. It's the specific combination of `if' and the `(('
> command that causes the problem.

Looks like same thing also happens when `if' is followed by a newline

./bash -c $'case $LINENO in 0) if\n:; then echo FAIL; fi esac'
bash/parse.y:980:82: runtime error: index -1 out of bounds for type 'int[257]'
#0 0x100759bcc in yyparse parse.y:980



asan report in extmatch

2023-03-16 Thread Grisha Levit
The relevant code was added in
https://git.savannah.gnu.org/cgit/bash.git/commit/?id=da43077 with
similar additions to both gmatch and extmatch, but I suspect the test
on line 912 was not meant to be in extmatch:

> .a
bash -O extglob -O dotglob -c ': ./!(.foo)'

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000102e02daf
READ of size 1 at 0x000102e02daf thread T0
#0 extmatch sm_loop.c:912

frame #5: bash`extmatch(xc=33, s=".a", se="", p="(.foo)", pe="",
flags=161) at sm_loop.c:912:36
   909
   910   if (m1 == 0 && (flags & FNM_DOTDOT) &&
   911   (SDOT_OR_DOTDOT (s) ||
-> 912((flags & FNM_PATHNAME) && s[-1] == L('/') && PDOT_OR_DOTDOT(s
   913 return (FNM_NOMATCH);
   914
   915   /* if srest > s, we are not at start of string */