syntax check of 'echo $HOME' crashes in ksh emulation mode

syntax check of 'echo $HOME' crashes in ksh emulation mode

[Kamil Dudka]

The following command causes a SIGSEGV in zsh (built from upstream git HEAD):

$ ARGV0=ksh zsh -nc 'echo $HOME'
zsh: segmentation fault (core dumped)  ARGV0=ksh Src/zsh -nc 'echo $HOME'

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000000004a3ded in sepsplit (s=0x0, sep=0x0, allownull=0, heap=1) at utils.c:3495
3495        if (s[0] == Nularg && !s[1])

(gdb) bt
#0  0x00000000004a3ded in sepsplit (s=0x0, sep=0x0, allownull=0, heap=1) at utils.c:3495
#1  0x0000000000497927 in paramsubst (l=0x7f63e07981a8, n=0x7f63e07981d8, str=0x7ffc3c5300f0, qt=0, pf_flags=16) at subst.c:3375
#2  0x0000000000490d4b in stringsubst (list=0x7f63e07981a8, node=0x7f63e07981d8, pf_flags=16, asssub=0) at subst.c:236
#3  0x0000000000490361 in prefork (list=0x7f63e07981a8, flags=0) at subst.c:77
#4  0x000000000042ed4e in execcmd (state=0x7ffc3c530bd0, input=0, output=0, how=18, last1=1) at exec.c:2724
#5  0x000000000042c391 in execpline2 (state=0x7ffc3c530bd0, pcode=131, how=18, input=0, output=0, last1=1) at exec.c:1748
#6  0x000000000042b2d0 in execpline (state=0x7ffc3c530bd0, slcode=4098, how=18, last1=1) at exec.c:1526
#7  0x000000000042a704 in execlist (state=0x7ffc3c530bd0, dont_change_job=0, exiting=1) at exec.c:1284
#8  0x0000000000429f4d in execode (p=0x7f63e0798130, dont_change_job=0, exiting=1, context=0x4b10e1 "cmdarg") at exec.c:1075
#9  0x0000000000429e24 in execstring (s=0x7ffc3c532223 "echo $HOME", dont_change_job=0, exiting=1, context=0x4b10e1 "cmdarg") at exec.c:1041
#10 0x000000000044dc73 in init_misc () at init.c:1281
#11 0x000000000044edf3 in zsh_main (argc=3, argv=0x7ffc3c530da8) at init.c:1664
#12 0x000000000040f296 in main (argc=3, argv=0x7ffc3c530da8) at ./main.c:93

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Peter Stephenson]

On Mon, 5 Oct 2015 18:09:22 +0200
Kamil Dudka <kdudka@redhat.com> wrote:
> The following command causes a SIGSEGV in zsh (built from upstream git HEAD):
> 
> $ ARGV0=ksh zsh -nc 'echo $HOME'
> zsh: segmentation fault (core dumped)  ARGV0=ksh Src/zsh -nc 'echo $HOME'

I'm not sure if we need to be more careful in paramsubst(), too, but if
we're not setting HOME because this is ksh emulation we should certainly
say so.

pws

diff --git a/Src/params.c b/Src/params.c
index de151a4..a8abb28 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -775,17 +775,18 @@ createparamtable(void)
 #endif
     opts[ALLEXPORT] = oae;
 
+    /*
+     * For native emulation we always set the variable home
+     * (see setupvals()).
+     */
+    pm = (Param) paramtab->getnode(paramtab, "HOME");
     if (EMULATION(EMULATE_ZSH))
     {
-	/*
-	 * For native emulation we always set the variable home
-	 * (see setupvals()).
-	 */
-	pm = (Param) paramtab->getnode(paramtab, "HOME");
 	pm->node.flags &= ~PM_UNSET;
 	if (!(pm->node.flags & PM_EXPORTED))
 	    addenv(pm, home);
-    }
+    } else if (!home)
+	pm->node.flags |= PM_UNSET;
     pm = (Param) paramtab->getnode(paramtab, "LOGNAME");
     if (!(pm->node.flags & PM_EXPORTED))
 	addenv(pm, pm->u.str);

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Bart Schaefer]

On Oct 5,  5:42pm, Peter Stephenson wrote:
}
} I'm not sure if we need to be more careful in paramsubst(), too, but if
} we're not setting HOME because this is ksh emulation we should certainly
} say so.

Hrm.  But we DO set HOME in ksh emulation if the -n option is not given.
What is it about no_exec that changes things?

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Axel Beckert]

Hi,

On Mon, Oct 05, 2015 at 06:09:22PM +0200, Kamil Dudka wrote:
> The following command causes a SIGSEGV in zsh (built from upstream git HEAD):
> 
> $ ARGV0=ksh zsh -nc 'echo $HOME'
> zsh: segmentation fault (core dumped)  ARGV0=ksh Src/zsh -nc 'echo $HOME'

JFTR: While I can reproduce this in zsh 5.0.7 and 4.3.17, I can't
reproduce this in zsh 4.3.10. (Just tried all zsh versions released
with still supported Debian Stable releases.)

		Kind regards, Axel
-- 
/~\  Plain Text Ribbon Campaign                   | Axel Beckert
\ /  Say No to HTML in E-Mail and News            | abe@deuxchevaux.org  (Mail)
 X   See http://www.nonhtmlmail.org/campaign.html | abe@noone.org (Mail+Jabber)
/ \  I love long mails: http://email.is-not-s.ms/ | http://abe.noone.org/ (Web)

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Peter Stephenson]

On Mon, 5 Oct 2015 10:45:07 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Hrm.  But we DO set HOME in ksh emulation if the -n option is not given.
> What is it about no_exec that changes things?

Not importing environment variables.

% ARGV0=ksh zsh -fc 'echo $HOME'
/export/home/pws
% (unset HOME; ARGV0=ksh zsh -fc 'echo $HOME')

%

pws

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Peter Stephenson]

On Tue, 6 Oct 2015 09:33:57 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Mon, 5 Oct 2015 10:45:07 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > Hrm.  But we DO set HOME in ksh emulation if the -n option is not given.
> > What is it about no_exec that changes things?
> 
> Not importing environment variables.

Digging deeper, I think where it's not doing the assignment is...


/**/
mod_export void
assignstrvalue(Value v, char *val, int flags)
{
    if (unset(EXECOPT))
	return;


(i) Doesn't that leak val?  Presumably if val is non-NULL it's
permanently allocated, and I don't think we can guarantee that never
happens.  Indeed, as far as I can see in the case we're talking about it
does happen.

(ii) It's not clear how safe the above is, as this case shows.  However,
it's definitely not safe to skip the tests in assignstrvalue() either,
and if we run them it's doing too much work for syntax checking and is
likely to mess things up owing to previous non-execution.  So this is all a
bit nasty.  (Just for once.)

pws

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Bart Schaefer]

On Oct 6, 10:15am, Peter Stephenson wrote:
}
} /**/
} mod_export void
} assignstrvalue(Value v, char *val, int flags)
} {
}     if (unset(EXECOPT))
} 	return;
} 
} (i) Doesn't that leak val?

Probably, but in no_exec state the shell can't possibly run for very long
so it doesn't matter?  However, every other case in assignstrvalue() has
zsfree(val) so this one probably should too.  Except:

} (ii) It's not clear how safe the above is, as this case shows.  However,
} it's definitely not safe to skip the tests in assignstrvalue() either

Do you mean that even for no_exec we should not be bailing out this
early in assignstrvalue()?  That is, we ought to be doing the tests of
read-only, wrong type, invalid subscript, etc., and then bail just
before removing the PM_UNSET flag?  Except:

} and if we run them it's doing too much work for syntax checking and is
} likely to mess things up owing to previous non-execution.

I don't know about too much work, but yes, some of them proceed a bit
too far into semantics.

} So this is all a bit nasty.  (Just for once.)

Does POSIX say anything about the environment in no_exec state?  Maybe
we should just temporarily toggle EXECOPT off during that part of the
shell initialization -- we're already avoiding execution of contents
of environment vars for security reasons, so it should be safe to do
the imports.

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Peter Stephenson]

On Tue, 6 Oct 2015 08:34:46 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> } (ii) It's not clear how safe the above is, as this case shows.  However,
> } it's definitely not safe to skip the tests in assignstrvalue() either
> 
> Do you mean that even for no_exec we should not be bailing out this
> early in assignstrvalue()?

It's not really clear, but ending up with NULL values in parameters
seems a bad thing.  However, I dare see we can fix things up as the occur.

It's not very helpful.

    The shell shall read commands but does not execute them; this can be
    used to check for shell script syntax errors. An interactive shell
    may ignore this option.

But I think there is an argument for setting environment variables in
full.

pws

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Peter Stephenson]

On Tue, 6 Oct 2015 20:46:54 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> It's not very helpful.
  ^^^^ i.e. POSIX, in case it's not clear.  Somehow the quotation
  disappeared.

>     The shell shall read commands but does not execute them; this can be
>     used to check for shell script syntax errors. An interactive shell
>     may ignore this option.
> 
> But I think there is an argument for setting environment variables in
> full.

pws

Re: syntax check of 'echo $HOME' crashes in ksh emulation mode

[Kamil Dudka]

On Monday 05 October 2015 17:42:19 Peter Stephenson wrote:
> On Mon, 5 Oct 2015 18:09:22 +0200
> 
> Kamil Dudka <kdudka@redhat.com> wrote:
> > The following command causes a SIGSEGV in zsh (built from upstream git
> > HEAD):
> > 
> > $ ARGV0=ksh zsh -nc 'echo $HOME'
> > zsh: segmentation fault (core dumped)  ARGV0=ksh Src/zsh -nc 'echo $HOME'
> 
> I'm not sure if we need to be more careful in paramsubst(), too, but if
> we're not setting HOME because this is ksh emulation we should certainly
> say so.
> 
> pws

Thanks for the patch!  I will get it included in Fedora/RHEL packages.

Kamil

> diff --git a/Src/params.c b/Src/params.c
> index de151a4..a8abb28 100644
> --- a/Src/params.c
> +++ b/Src/params.c
> @@ -775,17 +775,18 @@ createparamtable(void)
>  #endif
>      opts[ALLEXPORT] = oae;
> 
> +    /*
> +     * For native emulation we always set the variable home
> +     * (see setupvals()).
> +     */
> +    pm = (Param) paramtab->getnode(paramtab, "HOME");
>      if (EMULATION(EMULATE_ZSH))
>      {
> -	/*
> -	 * For native emulation we always set the variable home
> -	 * (see setupvals()).
> -	 */
> -	pm = (Param) paramtab->getnode(paramtab, "HOME");
>  	pm->node.flags &= ~PM_UNSET;
>  	if (!(pm->node.flags & PM_EXPORTED))
>  	    addenv(pm, home);
> -    }
> +    } else if (!home)
> +	pm->node.flags |= PM_UNSET;
>      pm = (Param) paramtab->getnode(paramtab, "LOGNAME");
>      if (!(pm->node.flags & PM_EXPORTED))
>  	addenv(pm, pm->u.str);