* Zsh - Multiple DoS Vulnerabilities @ 2019-05-10 15:03 David Wells 2019-05-10 16:37 ` Bart Schaefer 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer 0 siblings, 2 replies; 34+ messages in thread From: David Wells @ 2019-05-10 15:03 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 2117 bytes --] Hello Zsh-Workers, Tenable has discovered multiple DoS vulnerabilities in Zsh. The root cause appears to be Invalid Memory Access issues that crash the Zsh runtime. We believe these have the following Cvss v2 vector: AV:L/AC:L/Au:S/C:N/I:N/A:C and verified this is present when installed on Arch Linux 5.0.7 x64. We've internally assigned this vulnerability TRA-221. Here is a link where you'll find a proof of concept (PoC) called *zsh_poc.tar.bz2: * https://tenable.box.com/s/mi7vlmqgq5zpqhadlr90u2hit2r1hjwe. The PoC contains the 7 different invalid memory access issues in their respective directory. Each directory will contain a gdb stack trace as well as the Zsh script which can trigger the bug. #1 Invalid read from *taddrstr *call in *text.c* POC folder: *01_taddstr_(text.c_148)* #2 Invalid read from *execcmd_analyse *in *exec.c* POC folder: *02_execcmd_analyse_(exec.c_3653)* #3 Invalid read from *dupstring *in *string.c* POC folder: *03_dupstring_(string.c_39)* #4 Invalid read from *bin_print *in *builtin.c* POC folder: *04_bin_print_(builtin.c_5009)* #5 Invalid read from *untokenize *in *exec.c* POC folder: *05_untokenize_(exec.c_1994)* #6 Invalid read from *getjob *in *jobs.c* POC folder: *06_getjob_(jobs.c_1935)* #7 Invalid read from *hasher *in *hashtable.c* POC folder: *07_hasher_(hashtable.c_85)* Tenable follows a 90-day vulnerability disclosure policy. That means, even though we prefer coordinated disclosure, we'll issue an advisory on *August 8, 2019 *with or without a patch. Alternatively, any uncoordinated patch publicly released before the 90-day deadline will be considered public disclosure, and Tenable may release an early advisory. You can read the full details of our policy here: https://static.tenable.com/research/tenable-vulnerability-disclosure-policy.pdf Thank you for taking the time to read this. We'd greatly appreciate it if you'd acknowledge receipt of this report. If you have any questions we'd be happy to address them. Thanks again, David ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-10 15:03 Zsh - Multiple DoS Vulnerabilities David Wells @ 2019-05-10 16:37 ` Bart Schaefer 2019-05-12 16:21 ` Stephane Chazelas 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer 1 sibling, 1 reply; 34+ messages in thread From: Bart Schaefer @ 2019-05-10 16:37 UTC (permalink / raw) To: David Wells; +Cc: zsh-workers It would be helpful if you could explain how this would be exploited by someone who is not already able to cause the zsh user to execute some other arbitrary commands. What's the point of crashing somebody's shell if you can instead make it remove all their files or email you their private ssh keys or something? ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-10 16:37 ` Bart Schaefer @ 2019-05-12 16:21 ` Stephane Chazelas 2019-05-13 16:29 ` David Wells 2019-05-31 12:05 ` [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) Stephane Chazelas 0 siblings, 2 replies; 34+ messages in thread From: Stephane Chazelas @ 2019-05-12 16:21 UTC (permalink / raw) To: Bart Schaefer; +Cc: David Wells, zsh-workers 2019-05-10 09:37:15 -0700, Bart Schaefer: > It would be helpful if you could explain how this would be exploited > by someone who is not already able to cause the zsh user to execute > some other arbitrary commands. What's the point of crashing > somebody's shell if you can instead make it remove all their files or > email you their private ssh keys or something? I'd second that. I suspect it's shellshock aftermath. Before shellshock, the bash parser was invoked on the content of any environment variable that started with (), so on attacker supplied data as opposed to code of script authors. bash initially didn't check that the content of the variable was limited to only a function body. After that bug was fixed, the bash parser was still exposed to arbitrary env vars, and so people started "fuzzing" the bash parser to discover vulnerabilities that could be exploited. Quickly, bash was fixed so that it was not exposed to arbitrary variables, so most of those vulnerabilities in the parser became minor bugs. A few of those were still found long after shellshock was fixed though. I can't see David's reports, but they do sound like similar fuzzing test reports. After shellshock a few related vulnerabilities were found in zsh, where for instance the zsh arithmetic expression parser (and by extension the full parser) was invoked on the content of *some* specific env vars (like SHLVL). That's Oliver's (IIRC) $ SHLVL='psvar[0$(uname>&2)]' zsh -c '' Linux But those were fixed. Nowadays, I don't think zsh's parser is invoked on attacker supplied-data, and if it were, the fact that the code crashes the shell would be, as you say, the least of our worries. There is one exception I can think of though: the restricted mode. That's one mode where the attacker can be the shell code author. If those bugs allow the user to escape the restricted shell jail, then it would be a concern. If however the only problem is the crashing of the shell, then it would not be a concern, as the user can always do "kill -s SEGV $$" to crash the shell (or set and reach some limits, or do a deep recursion...). Now, I don't think we need those bugs to escape a restricted shell jails as those are almost impossible to implement reliably and are an anachronism nowadays IMO. I would be of the opinion that that feature should be removed (I'd be curious to know if anybody has ever used it). At least, we should give more warning about it and recommend alternatives. Here's an attempt below: diff --git a/Doc/Zsh/restricted.yo b/Doc/Zsh/restricted.yo index 6cf9b36b5..121e2ae8d 100644 --- a/Doc/Zsh/restricted.yo +++ b/Doc/Zsh/restricted.yo @@ -37,3 +37,46 @@ Restricted mode can also be activated any time by setting the tt(RESTRICTED) option. This immediately enables all the restrictions described above even if the shell still has not processed all startup files. + +A shell em(Restricted Mode) is an ancient way to restrict what users may +do. However modern systems have better, safer and more reliable ways to +confine user actions like em(chroot jails), em(containers) or em(zones). + +A restricted shell is very difficult to implement safely. That feature +may be removed in a future version of zsh. + +It's important to realise the restrictions only apply to the shell and +not to the commands it runs (except for some of its builtins). While a +restricted shell can only run the restricted list of commands accessible +via the predefined `tt(PATH)` variable, it doesn't prevent those +commands from running any other command. + +As an example, if `tt(env)' is among the list of em(allowed) commands, +then it allows the user to run any command as `tt(env)` is not a shell +builtin command and can run arbitrary executables. + +So when implementing a restricted shell framework it's important to be +fully aware of what actions each of the em(allowed) commands or features +(think em(modules)) can perform. + +Many commands can have their behaviour affected by environment +variables. Except for the few listed above, zsh doesn't restrict setting +environment variables. + +Having a `tt(perl)', `tt(python)', `tt(bash)` script as a restricted +command probably means the user can work around the restriction by +setting specially crafted `tt(PERL5LIB)', `tt(PYTHONPATH)', +`tt(BASHENV)' environment variables. On GNU systems, one can have any +command doing character set conversion (which includes zsh itself) run +arbitrary code by setting a `tt(GCONV_PATH)' environment variable, those +are only a few examples. + +Bear in mind that contrary to some other shells, `tt(readonly)' is not a +security feature in zsh as it can be undone and so cannot be used to +mitigate the above. + +A restricted shell is only going to work if the allowed commands are few +and carefully written so as not to grant more access to users than +intended. It's also important to restrict what zsh module the user may +load as some of them like `tt(zsh/system)', `tt(zsh/mapfile)' or +`tt(zsh/files)' would allow bypassing most of the restrictions. -- Stephane ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-12 16:21 ` Stephane Chazelas @ 2019-05-13 16:29 ` David Wells 2019-05-13 22:02 ` Bart Schaefer 2019-05-14 18:10 ` Stephane Chazelas 2019-05-31 12:05 ` [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) Stephane Chazelas 1 sibling, 2 replies; 34+ messages in thread From: David Wells @ 2019-05-13 16:29 UTC (permalink / raw) To: David Wells, Bart Schaefer, zsh-workers [-- Attachment #1: Type: text/plain, Size: 5947 bytes --] Thanks for taking a look at these bugs. As Stephanie mentioned, security related risk may depend more on Zsh usage, and being that these crashes are Invalid Memory Access issues, they might allow an attacker to disclose parts of memory to help with a pre-exploitation process. It looks like there is patch activity on this thread, would you be able to provide me update on expected patch date and issues you are patching? Thank you. On Sun, May 12, 2019 at 9:21 AM Stephane Chazelas < stephane.chazelas@gmail.com> wrote: > 2019-05-10 09:37:15 -0700, Bart Schaefer: > > It would be helpful if you could explain how this would be exploited > > by someone who is not already able to cause the zsh user to execute > > some other arbitrary commands. What's the point of crashing > > somebody's shell if you can instead make it remove all their files or > > email you their private ssh keys or something? > > I'd second that. > > I suspect it's shellshock aftermath. > > Before shellshock, the bash parser was invoked on the content of any > environment variable that started with (), so on attacker > supplied data as opposed to code of script authors. > > bash initially didn't check that the content of the variable was > limited to only a function body. After that bug was fixed, the > bash parser was still exposed to arbitrary env vars, and so > people started "fuzzing" the bash parser to discover > vulnerabilities that could be exploited. Quickly, bash was fixed > so that it was not exposed to arbitrary variables, so most of > those vulnerabilities in the parser became minor bugs. > > A few of those were still found long after shellshock was fixed > though. > > I can't see David's reports, but they do sound like similar > fuzzing test reports. > > After shellshock a few related vulnerabilities were found in > zsh, where for instance the zsh arithmetic expression parser > (and by extension the full parser) was invoked on the content of > *some* specific env vars (like SHLVL). That's Oliver's (IIRC) > > $ SHLVL='psvar[0$(uname>&2)]' zsh -c '' > Linux > > But those were fixed. Nowadays, I don't think zsh's parser is > invoked on attacker supplied-data, and if it were, the fact that > the code crashes the shell would be, as you say, the least of > our worries. > > There is one exception I can think of though: the restricted > mode. > > That's one mode where the attacker can be the shell code author. > > If those bugs allow the user to escape the restricted shell > jail, then it would be a concern. If however the only problem is > the crashing of the shell, then it would not be a concern, as > the user can always do "kill -s SEGV $$" to crash the shell (or > set and reach some limits, or do a deep recursion...). > > Now, I don't think we need those bugs to escape a restricted > shell jails as those are almost impossible to implement > reliably and are an anachronism nowadays IMO. > > I would be of the opinion that that feature should be removed > (I'd be curious to know if anybody has ever used it). > > At least, we should give more warning about it and recommend > alternatives. Here's an attempt below: > > diff --git a/Doc/Zsh/restricted.yo b/Doc/Zsh/restricted.yo > index 6cf9b36b5..121e2ae8d 100644 > --- a/Doc/Zsh/restricted.yo > +++ b/Doc/Zsh/restricted.yo > @@ -37,3 +37,46 @@ Restricted mode can also be activated any time by > setting the > tt(RESTRICTED) option. This immediately enables all the restrictions > described above even if the shell still has not processed all startup > files. > + > +A shell em(Restricted Mode) is an ancient way to restrict what users may > +do. However modern systems have better, safer and more reliable ways to > +confine user actions like em(chroot jails), em(containers) or em(zones). > + > +A restricted shell is very difficult to implement safely. That feature > +may be removed in a future version of zsh. > + > +It's important to realise the restrictions only apply to the shell and > +not to the commands it runs (except for some of its builtins). While a > +restricted shell can only run the restricted list of commands accessible > +via the predefined `tt(PATH)` variable, it doesn't prevent those > +commands from running any other command. > + > +As an example, if `tt(env)' is among the list of em(allowed) commands, > +then it allows the user to run any command as `tt(env)` is not a shell > +builtin command and can run arbitrary executables. > + > +So when implementing a restricted shell framework it's important to be > +fully aware of what actions each of the em(allowed) commands or features > +(think em(modules)) can perform. > + > +Many commands can have their behaviour affected by environment > +variables. Except for the few listed above, zsh doesn't restrict setting > +environment variables. > + > +Having a `tt(perl)', `tt(python)', `tt(bash)` script as a restricted > +command probably means the user can work around the restriction by > +setting specially crafted `tt(PERL5LIB)', `tt(PYTHONPATH)', > +`tt(BASHENV)' environment variables. On GNU systems, one can have any > +command doing character set conversion (which includes zsh itself) run > +arbitrary code by setting a `tt(GCONV_PATH)' environment variable, those > +are only a few examples. > + > +Bear in mind that contrary to some other shells, `tt(readonly)' is not a > +security feature in zsh as it can be undone and so cannot be used to > +mitigate the above. > + > +A restricted shell is only going to work if the allowed commands are few > +and carefully written so as not to grant more access to users than > +intended. It's also important to restrict what zsh module the user may > +load as some of them like `tt(zsh/system)', `tt(zsh/mapfile)' or > +`tt(zsh/files)' would allow bypassing most of the restrictions. > > -- > Stephane > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-13 16:29 ` David Wells @ 2019-05-13 22:02 ` Bart Schaefer 2019-05-14 18:10 ` Stephane Chazelas 1 sibling, 0 replies; 34+ messages in thread From: Bart Schaefer @ 2019-05-13 22:02 UTC (permalink / raw) To: David Wells; +Cc: zsh-workers On Mon, May 13, 2019 at 9:29 AM David Wells <bughunters@tenable.com> wrote: > > Thanks for taking a look at these bugs. As Stephanie mentioned, security related risk may depend more on Zsh usage, and being that these crashes are Invalid Memory Access issues, they might allow an attacker to disclose parts of memory to help with a pre-exploitation process. It looks like there is patch activity on this thread, would you be able to provide me update on expected patch date and issues you are patching? Thank you. It's Stephane, not Stephanie. :-) Zsh support is entirely by volunteers, there's no one with time dedicated to this. It looks like Oliver may be tackling a number of these, but there's no way for any of us to assert or predict a date when any particular bug will get worked on or when a release will be made. (Please note that you're already testing a pre-release version as it is.) ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-13 16:29 ` David Wells 2019-05-13 22:02 ` Bart Schaefer @ 2019-05-14 18:10 ` Stephane Chazelas 2019-05-14 21:24 ` Daniel Shahaf 2019-05-14 21:39 ` Daniel Shahaf 1 sibling, 2 replies; 34+ messages in thread From: Stephane Chazelas @ 2019-05-14 18:10 UTC (permalink / raw) To: David Wells; +Cc: Bart Schaefer, zsh-workers 2019-05-13 09:29:36 -0700, David Wells: > Thanks for taking a look at these bugs. As Stephanie mentioned, security > related risk may depend more on Zsh usage, and being that these crashes are > Invalid Memory Access issues, they might allow an attacker to disclose > parts of memory to help with a pre-exploitation process. [...] Thanks David, I think it's fair to say you've flagged those as DoS vulnerabilities solely on the ground they were "invalid memory access issues" irrespective of how exploitable they are. I'd say those reports are useful as bug reports as they point to areas where zsh isn't doing the right thing, and I'm sure zsh developers are grateful for it. However I think it does a disservice (to the sysadmin and security community (your customers as I understand your company creates vulnerability scanning software), not so much to zsh developers) to call them DoS vulnerabilities as they are not. It's a shell's (or any interpreter's for that matters) expected behaviour to fail when provided with unexpected input, so, that the shell crashes upon invalid input is not really a problem. A shell will exit when given "exit" or "<()>" as input, that doesn't mean it's a DoS. Also, it's a shells job to execute arbitrary command. A :(){ :|:&};: input (the infamous fork bomb) will cripple a system, but that's as intended by whoever wrote that code. From what I can see, those reported issues would only be vulnerabilities if they were a way to escape the "restricted mode" of zsh, a "security" feature inherited from the Bourne shell which hopefully nobody uses these days, but not as DoS vulnerabilities. I think you should change the vectors. IMO, from a security standpoint, it's not very useful to fuzz "code" input provided to zsh, as anyway any "code" allows zsh to run any arbitrary command (except for the restricted mode). In other words, the "code" is generally not the attacker supplied data. You could fuzz environment variables (the ones zsh cares about) or other attacker-controlled data fed to zsh scripts like "limits" instead. -- Stephane ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 18:10 ` Stephane Chazelas @ 2019-05-14 21:24 ` Daniel Shahaf 2019-05-14 21:38 ` Bart Schaefer 2019-05-14 21:39 ` Daniel Shahaf 1 sibling, 1 reply; 34+ messages in thread From: Daniel Shahaf @ 2019-05-14 21:24 UTC (permalink / raw) To: David Wells; +Cc: zsh-workers Stephane Chazelas wrote on Tue, 14 May 2019 18:11 +00:00: > IMO, from a security standpoint, it's not very useful to fuzz > "code" input provided to zsh, as anyway any "code" allows zsh to > run any arbitrary command (except for the restricted mode). In > other words, the "code" is generally not the attacker supplied > data. Sounds right. There might be some corner case here > You could fuzz environment variables (the ones zsh cares > about) or other attacker-controlled data fed to zsh scripts like > "limits" instead. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 21:24 ` Daniel Shahaf @ 2019-05-14 21:38 ` Bart Schaefer 0 siblings, 0 replies; 34+ messages in thread From: Bart Schaefer @ 2019-05-14 21:38 UTC (permalink / raw) To: Daniel Shahaf; +Cc: David Wells, zsh-workers On Tue, May 14, 2019 at 2:25 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Stephane Chazelas wrote on Tue, 14 May 2019 18:11 +00:00: > > IMO, from a security standpoint, it's not very useful to fuzz > > "code" input provided to zsh, as anyway any "code" allows zsh to > > run any arbitrary command (except for the restricted mode). In > > other words, the "code" is generally not the attacker supplied > > data. > > Sounds right. There might be some corner case here The other interesting case would be one where the zsh code enabled privilege escalation, i.e., where the coder is the attacker and the shell is the vector to a different security issue. A zsh script to exploit ZombieLoad, for example. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 18:10 ` Stephane Chazelas 2019-05-14 21:24 ` Daniel Shahaf @ 2019-05-14 21:39 ` Daniel Shahaf 2019-05-14 22:25 ` Bart Schaefer 1 sibling, 1 reply; 34+ messages in thread From: Daniel Shahaf @ 2019-05-14 21:39 UTC (permalink / raw) To: David Wells; +Cc: zsh-workers [sorry for double send] Stephane Chazelas wrote on Tue, 14 May 2019 18:11 +00:00: > IMO, from a security standpoint, it's not very useful to fuzz > "code" input provided to zsh, as anyway any "code" allows zsh to > run any arbitrary command (except for the restricted mode). In > other words, the "code" is generally not the attacker supplied > data. Sounds right. I've been trying to come up with counterexamples. What if somebody installed a /etc/zshenv that does, say, 'disable zmodload enable'? If that actually prevents zmodload from being run,¹ then a bug that allows zmodload to be run would be interesting. Cheers, Daniel ¹ I'm not sure it does because there might be some other way to run zmodload — an assignment to $modules, maybe? (Don't have time to test this, sorry.) ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 21:39 ` Daniel Shahaf @ 2019-05-14 22:25 ` Bart Schaefer 2019-05-15 10:48 ` Daniel Shahaf 0 siblings, 1 reply; 34+ messages in thread From: Bart Schaefer @ 2019-05-14 22:25 UTC (permalink / raw) To: Daniel Shahaf; +Cc: David Wells, zsh-workers On Tue, May 14, 2019 at 2:39 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > I've been trying to come up with counterexamples. What if somebody > installed a /etc/zshenv that does, say, 'disable zmodload enable'? You can bypass /etc/zshenv by, for example, invoking zsh as "sh" and then running "emulate -R" and/or otherwise futzing with setopts. So either THAT is a security flaw, or your example isn't one either. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 22:25 ` Bart Schaefer @ 2019-05-15 10:48 ` Daniel Shahaf 0 siblings, 0 replies; 34+ messages in thread From: Daniel Shahaf @ 2019-05-15 10:48 UTC (permalink / raw) To: zsh-workers; +Cc: David Wells Bart Schaefer wrote on Tue, 14 May 2019 22:26 +00:00: > On Tue, May 14, 2019 at 2:39 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > I've been trying to come up with counterexamples. What if somebody > > installed a /etc/zshenv that does, say, 'disable zmodload enable'? > > You can bypass /etc/zshenv by, for example, invoking zsh as "sh" and > then running "emulate -R" and/or otherwise futzing with setopts. I don't think there's an easy solution here, since sourcing /etc/zshenv in mid-session could be a can of worms, too. > So either THAT is a security flaw, or your example isn't one either. I suppose my example was a security flaw _in the sysadmin's setup_. If someone wants to make the case that it's a bug in zsh, I'm all ears. Cheers, Daniel ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) 2019-05-12 16:21 ` Stephane Chazelas 2019-05-13 16:29 ` David Wells @ 2019-05-31 12:05 ` Stephane Chazelas 2019-06-03 9:35 ` Peter Stephenson 2019-06-04 2:39 ` dana 1 sibling, 2 replies; 34+ messages in thread From: Stephane Chazelas @ 2019-05-31 12:05 UTC (permalink / raw) To: Bart Schaefer, David Wells, zsh-workers 2019-05-12 17:21:49 +0100, Stephane Chazelas: [...] > At least, we should give more warning about it and recommend > alternatives. Here's an attempt below: I'm bringing this up again as it looks like it has stayed mostly unnoticed (except maybe by Chet ;-)) IMO, the restricted mode should be deprecated. In any case, we should at least warn against using it. Below was my attempt which could definitely be improved, I hope it can serve as a basic for discussio. See also what Chet recently added about it in the bash documentation at https://git.savannah.gnu.org/cgit/bash.git/commit/doc/bash.info?id=52e469696418877c5b7cd7f752f5f8309ef65a38 which is more concise. diff --git a/Doc/Zsh/restricted.yo b/Doc/Zsh/restricted.yo index 6cf9b36b5..121e2ae8d 100644 --- a/Doc/Zsh/restricted.yo +++ b/Doc/Zsh/restricted.yo @@ -37,3 +37,46 @@ Restricted mode can also be activated any time by setting the tt(RESTRICTED) option. This immediately enables all the restrictions described above even if the shell still has not processed all startup files. + +A shell em(Restricted Mode) is an ancient way to restrict what users may +do. However modern systems have better, safer and more reliable ways to +confine user actions like em(chroot jails), em(containers) or em(zones). + +A restricted shell is very difficult to implement safely. That feature +may be removed in a future version of zsh. + +It's important to realise the restrictions only apply to the shell and +not to the commands it runs (except for some of its builtins). While a +restricted shell can only run the restricted list of commands accessible +via the predefined `tt(PATH)` variable, it doesn't prevent those +commands from running any other command. + +As an example, if `tt(env)' is among the list of em(allowed) commands, +then it allows the user to run any command as `tt(env)` is not a shell +builtin command and can run arbitrary executables. + +So when implementing a restricted shell framework it's important to be +fully aware of what actions each of the em(allowed) commands or features +(think em(modules)) can perform. + +Many commands can have their behaviour affected by environment +variables. Except for the few listed above, zsh doesn't restrict setting +environment variables. + +Having a `tt(perl)', `tt(python)', `tt(bash)` script as a restricted +command probably means the user can work around the restriction by +setting specially crafted `tt(PERL5LIB)', `tt(PYTHONPATH)', +`tt(BASHENV)' environment variables. On GNU systems, one can have any +command doing character set conversion (which includes zsh itself) run +arbitrary code by setting a `tt(GCONV_PATH)' environment variable, those +are only a few examples. + +Bear in mind that contrary to some other shells, `tt(readonly)' is not a +security feature in zsh as it can be undone and so cannot be used to +mitigate the above. + +A restricted shell is only going to work if the allowed commands are few +and carefully written so as not to grant more access to users than +intended. It's also important to restrict what zsh module the user may +load as some of them like `tt(zsh/system)', `tt(zsh/mapfile)' or +`tt(zsh/files)' would allow bypassing most of the restrictions. -- Stephane ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) 2019-05-31 12:05 ` [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) Stephane Chazelas @ 2019-06-03 9:35 ` Peter Stephenson 2019-06-04 2:39 ` dana 1 sibling, 0 replies; 34+ messages in thread From: Peter Stephenson @ 2019-06-03 9:35 UTC (permalink / raw) To: zsh-workers On Fri, 2019-05-31 at 13:05 +0100, Stephane Chazelas wrote: > 2019-05-12 17:21:49 +0100, Stephane Chazelas: > [...] > > > > At least, we should give more warning about it and recommend > > alternatives. Here's an attempt below: > I'm bringing this up again as it looks like it has stayed mostly > unnoticed (except maybe by Chet ;-)) > > IMO, the restricted mode should be deprecated. In any case, we > should at least warn against using it. Below was my attempt > which could definitely be improved, I hope it can serve as a > basic for discussio. It does seems reasonable to have something like this, but I'm not sure I'm feeling like I can do any particularly useful editing myself. If nobody has any immediate comments we can simply put the whole thing in, where it obviously remains available for any follow up changes. pws ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) 2019-05-31 12:05 ` [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) Stephane Chazelas 2019-06-03 9:35 ` Peter Stephenson @ 2019-06-04 2:39 ` dana 2019-06-04 7:34 ` dana 1 sibling, 1 reply; 34+ messages in thread From: dana @ 2019-06-04 2:39 UTC (permalink / raw) To: Stephane Chazelas; +Cc: zsh-workers Without bike-shedding too much, i did notice some trivial formatting and punctuation issues: On 31 May 2019, at 07:05, Stephane Chazelas <stephane.chazelas@gmail.com> wrote: > +via the predefined `tt(PATH)` variable, it doesn't prevent those The quote usage isn't consistent on these. AFAIR, the convention is for all of them to be `tt(...)' (tilde to open, single-quote to close) On 31 May 2019, at 07:05, Stephane Chazelas <stephane.chazelas@gmail.com> wrote: > +arbitrary code by setting a `tt(GCONV_PATH)' environment variable, those > +are only a few examples. 'Those are...' should start a new sentence On 31 May 2019, at 07:05, Stephane Chazelas <stephane.chazelas@gmail.com> wrote: > +Bear in mind that contrary to some other shells, `tt(readonly)' is not a 'Bear in mind that, contrary to other shells, ...' (add comma) dana ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) 2019-06-04 2:39 ` dana @ 2019-06-04 7:34 ` dana 0 siblings, 0 replies; 34+ messages in thread From: dana @ 2019-06-04 7:34 UTC (permalink / raw) To: Stephane Chazelas; +Cc: zsh-workers On 3 Jun 2019, at 21:39, dana <dana@dana.is> wrote: > The quote usage isn't consistent on these. AFAIR, the convention is for all of > them to be `tt(...)' (tilde to open, single-quote to close) I meant back-tick instead of tilde here, obv. Couldn't let that go dana ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-10 15:03 Zsh - Multiple DoS Vulnerabilities David Wells 2019-05-10 16:37 ` Bart Schaefer @ 2019-05-10 20:27 ` Bart Schaefer 2019-05-11 1:45 ` #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle ` (5 more replies) 1 sibling, 6 replies; 34+ messages in thread From: Bart Schaefer @ 2019-05-10 20:27 UTC (permalink / raw) To: David Wells; +Cc: zsh-workers On Fri, May 10, 2019 at 8:04 AM David Wells <bughunters@tenable.com> wrote: > > #1 Invalid read from *taddrstr *call in *text.c* > POC folder: *01_taddstr_(text.c_148)* This has literal NUL bytes embedded in the body of an if/then. Run from an interactive shell, it gives: text.c:995: unknown word code in gettext2() text.c:995: unknown word code in gettext2() text.c:72: attempting to decrement tindent below zero text.c:72: attempting to decrement tindent below zero and then (several seconds later) a crash. The following minimal subset of their test will put the shell into an infinite loop, without (at least for as long as I was willing to wait) crashing it: if true; then me > you || ! : fi > #2 Invalid read from *execcmd_analyse *in *exec.c* > POC folder: *02_execcmd_analyse_(exec.c_3653)* The test case is 3kb of a mangled shell script (missing closing quotes, random bytes inserted) so I'm not going to attempt to reduce it to a minimal case. Feeding it to "zsh -nf" yields: 11: exec.c:2655: BUG: miscounted typeset assignments 11: exec.c:2655: BUG: miscounted typeset assignments 11: exec.c:2655: BUG: miscounted typeset assignments 11: exec.c:2655: BUG: miscounted typeset assignments and then after several seconds a crash. I did not attempt feeding this (or #3 - #5) through a shell that does not have the -n option, because I don't have a secure sandbox in which to run scripts I can't visually verify. > #3 Invalid read from *dupstring *in *string.c* > POC folder: *03_dupstring_(string.c_39)* This gives exactly the same errors as #2, and then exits with [long ugly filename]:87: parse error near `}' > #4 Invalid read from *bin_print *in *builtin.c* > POC folder: *04_bin_print_(builtin.c_5009)* This produces no error output at all except for: [long ugly filename]:41: parse error near `)' > #5 Invalid read from *untokenize *in *exec.c* > POC folder: *05_untokenize_(exec.c_1994)* Again no error except: [long ugly filename]:94: parse error near `}' > #6 Invalid read from *getjob *in *jobs.c* > POC folder: *06_getjob_(jobs.c_1935)* This one I fed to "zsh -xf" and got (file name removed for readability): +1> bg $'%\M-\C-?' $'\C-VI7' bg:1: no job control in this shell. +1> disown $'%777777777777777\M-^' +1> $'\C-[' +1> $'\C-X\C-@\C-@\C-@@\C-@7' 1: command not found: ^[ 1: command not found: ^X followed eventually by a crash. The input has multiple NUL bytes following the ^X, and then some other misc. garbage, so the input processing may have a generic problem with NULs. > #7 Invalid read from *hasher *in *hashtable.c* > POC folder: *07_hasher_(hashtable.c_85)* For this one "zsh -xf" says: +1> foset :print $'\C-@\C-@\C-@hree' 1: command not found: foset +1> set -E e +2> typeset -priTt CeE e and then just goes away until killed. Only that final typeset is necessary to reproduce the bug, the rest is irrelevant. ^ permalink raw reply [flat|nested] 34+ messages in thread
* #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer @ 2019-05-11 1:45 ` Oliver Kiddle 2019-05-13 9:01 ` Peter Stephenson 2019-05-13 21:11 ` PATCH: #6 negative job id (Re: " Oliver Kiddle ` (4 subsequent siblings) 5 siblings, 1 reply; 34+ messages in thread From: Oliver Kiddle @ 2019-05-11 1:45 UTC (permalink / raw) To: Bart Schaefer; +Cc: David Wells, zsh-workers Bart wrote: > > #7 Invalid read from *hasher *in *hashtable.c* > > POC folder: *07_hasher_(hashtable.c_85)* > +2> typeset -priTt CeE e > > and then just goes away until killed. Only that final typeset is > necessary to reproduce the bug, the rest is irrelevant. Actually all it needs is the combination of -T and -p along with variables that aren't already tied. With other relevant branches of bin_typeset() for things like typeset -pi, we check for OPT_ISSET(ops, 'p') early and don't call typeset_single(). (-m is an exception to this). An alternative to the following patch would be to simply print an error. This would just involve the -p test joining the -m test on the next lines after the final bit of context. However, the behaviour as per this patch is more consistent with other meaningless typeset combinations like -pi and -pF. I'm fairly certain that the second part of the patch renders the lines removed in the first part as dead code but it'd be good if someone else could check my logic which is as follows: Given -p, typeset_single() is only now called when -m is set. usepm will always then be true because with -m, pm will always be set and never with PM_UNSET. So we go into the if (usepm) block on line 2193 which has early returns on every branch. Oliver diff --git a/Src/builtin.c b/Src/builtin.c index 49f017046..ca0ce35f5 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -2583,9 +2583,6 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), } pm->node.flags |= (on & PM_READONLY); - if (OPT_ISSET(ops,'p')) - paramtab->printnode(&pm->node, PRINT_TYPESET); - return pm; } @@ -2714,7 +2711,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func) (!isset(GLOBALEXPORT) && !OPT_ISSET(ops,'g'))) on |= PM_LOCAL; - if (on & PM_TIED) { + if ((on & PM_TIED) && !OPT_ISSET(ops, 'p')) { Param apm; struct asgment asg0, asg2; char *oldval = NULL, *joinstr; ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-11 1:45 ` #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle @ 2019-05-13 9:01 ` Peter Stephenson 0 siblings, 0 replies; 34+ messages in thread From: Peter Stephenson @ 2019-05-13 9:01 UTC (permalink / raw) To: zsh-workers On Sat, 2019-05-11 at 03:45 +0200, Oliver Kiddle wrote: > I'm fairly certain that the second part of the patch renders the lines > removed in the first part as dead code but it'd be good if someone else > could check my logic which is as follows: Given -p, typeset_single() is > only now called when -m is set. usepm will always then be true because > with -m, pm will always be set and never with PM_UNSET. So we go into > the if (usepm) block on line 2193 which has early returns on every > branch. That certainly looks plausible but you could always put a DPUTS() check there. pws ^ permalink raw reply [flat|nested] 34+ messages in thread
* PATCH: #6 negative job id (Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer 2019-05-11 1:45 ` #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle @ 2019-05-13 21:11 ` Oliver Kiddle 2019-05-13 21:44 ` Zsh - Multiple DoS Vulnerabilities Oliver Kiddle ` (3 subsequent siblings) 5 siblings, 0 replies; 34+ messages in thread From: Oliver Kiddle @ 2019-05-13 21:11 UTC (permalink / raw) To: David Wells, zsh-workers On 10 May, Bart wrote: > > #6 Invalid read from *getjob *in *jobs.c* > > POC folder: *06_getjob_(jobs.c_1935)* > > This one I fed to "zsh -xf" and got (file name removed for readability): > > +1> bg $'%\M-\C-?' $'\C-VI7' > bg:1: no job control in this shell. > +1> disown $'%777777777777777\M-^' This can be reproduced with just %777777777777777 or %2147483648 for that matter. Seems the value returned from atoi() wraps to negative values if it doesn't fit in an int. This patch prevents the crash but perhaps atoi() should be replaced with something that does better error handling to cover numbers that are too big but get truncated to something positive. Oliver diff --git a/Src/jobs.c b/Src/jobs.c index 73d7f26da..50751decb 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -1932,7 +1932,7 @@ getjob(const char *s, const char *prog) /* a digit here means we have a job number */ if (idigit(*s)) { jobnum = atoi(s); - if (jobnum && jobnum <= mymaxjob && myjobtab[jobnum].stat && + if (jobnum > 0 && jobnum <= mymaxjob && myjobtab[jobnum].stat && !(myjobtab[jobnum].stat & STAT_SUBJOB) && /* * If running jobs in a subshell, we are allowed to ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer 2019-05-11 1:45 ` #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle 2019-05-13 21:11 ` PATCH: #6 negative job id (Re: " Oliver Kiddle @ 2019-05-13 21:44 ` Oliver Kiddle 2019-05-13 22:36 ` #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle ` (2 subsequent siblings) 5 siblings, 0 replies; 34+ messages in thread From: Oliver Kiddle @ 2019-05-13 21:44 UTC (permalink / raw) To: David Wells, zsh-workers > On Fri, May 10, 2019 at 8:04 AM David Wells <bughunters@tenable.com> wrote: > > #4 Invalid read from *bin_print *in *builtin.c* > > POC folder: *04_bin_print_(builtin.c_5009)* This seems to be very similar to #6: string to int conversion overflowing to a negative number. In this case you can reproduce it with just: printf '%4444444444444$' Note that narg below is of type int despite the use of strtoul(). Oliver diff --git a/Src/builtin.c b/Src/builtin.c index ca0ce35f5..a8f054c8a 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -4990,8 +4990,7 @@ bin_print(char *name, char **args, Options ops, int func) narg = strtoul(c, &endptr, 0); if (*endptr == '$') { c = endptr + 1; - DPUTS(narg <= 0, "specified zero or negative arg"); - if (narg > argc) { + if (narg <= 0 || narg > argc) { zwarnnam(name, "%d: argument specifier out of range", narg); if (fout != stdout) ^ permalink raw reply [flat|nested] 34+ messages in thread
* #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer ` (2 preceding siblings ...) 2019-05-13 21:44 ` Zsh - Multiple DoS Vulnerabilities Oliver Kiddle @ 2019-05-13 22:36 ` Oliver Kiddle 2019-05-14 0:13 ` Mikael Magnusson 2019-05-14 10:50 ` Peter Stephenson 2019-05-14 16:38 ` Zsh - Multiple DoS Vulnerabilities Peter Stephenson 2019-05-14 20:30 ` Oliver Kiddle 5 siblings, 2 replies; 34+ messages in thread From: Oliver Kiddle @ 2019-05-13 22:36 UTC (permalink / raw) To: zsh-workers On 10 May, Bart wrote: > > #3 Invalid read from *dupstring *in *string.c* > > POC folder: *03_dupstring_(string.c_39)* > > This gives exactly the same errors as #2, and then exits with > > [long ugly filename]:87: parse error near `}' I've cut this one down to just: typeset Q= {X} That reliably seg faults for me. But that's about as far as I've been able to get - I'm not especially familiar with zsh's parsing code. Oliver ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-13 22:36 ` #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle @ 2019-05-14 0:13 ` Mikael Magnusson 2019-05-14 5:38 ` Bart Schaefer 2019-05-14 10:50 ` Peter Stephenson 1 sibling, 1 reply; 34+ messages in thread From: Mikael Magnusson @ 2019-05-14 0:13 UTC (permalink / raw) To: Oliver Kiddle; +Cc: zsh-workers On 5/14/19, Oliver Kiddle <okiddle@yahoo.co.uk> wrote: > On 10 May, Bart wrote: >> > #3 Invalid read from *dupstring *in *string.c* >> > POC folder: *03_dupstring_(string.c_39)* >> >> This gives exactly the same errors as #2, and then exits with >> >> [long ugly filename]:87: parse error near `}' > > I've cut this one down to just: > > typeset Q= {X} > > That reliably seg faults for me. But that's about as far as I've > been able to get - I'm not especially familiar with zsh's parsing > code. Yeah it looks like some stuff is not exactly going right here, Breakpoint 2, taddassign (code=0, state=0x7fffffffcfc0, typeset=1) at text.c:187 187 char *s = ecgetstr(state, EC_NODUP, NULL); (gdb) p *state $11 = {prog = 0x7ffff7ff23a8, pc = 0x7ffff7ff240c, strs = 0x7ffff7ff240c "typeset"} (gdb) step ecgetstr (s=0x7fffffffcfc0, dup=0, tokflag=0x0) at parse.c:2772 2772 wordcode c = *s->pc++; (gdb) 2775 if (c == 6 || c == 7) (gdb) 2777 else if (c & 2) { (gdb) p c $12 = 1701869940 (gdb) p *s $13 = {prog = 0x7ffff7ff23a8, pc = 0x7ffff7ff2410, strs = 0x7ffff7ff240c "typeset"} (gdb) p *s->pc $14 = 7628147 (gdb) p prog No symbol "prog" in current context. (gdb) p s->prog $15 = (Eprog) 0x7ffff7ff23a8 (gdb) p *s->prog $16 = {flags = 2, len = 52, npats = 0, nref = -1, pats = 0x7ffff7ff23e0, prog = 0x7ffff7ff23e0, strs = 0x7ffff7ff240c "typeset", shf = 0x0, dump = 0x0} (gdb) p *s->prog->prog $17 = 577 (gdb) p s->strs $18 = 0x7ffff7ff240c "typeset" (gdb) p s->strs+1 $19 = 0x7ffff7ff240d "ypeset" (gdb) list 2772 wordcode c = *s->pc++; 2773 char *r; 2774 2775 if (c == 6 || c == 7) 2776 r = ""; 2777 else if (c & 2) { 2778 buf[0] = (char) ((c >> 3) & 0xff); 2779 buf[1] = (char) ((c >> 11) & 0xff); 2780 buf[2] = (char) ((c >> 19) & 0xff); 2781 buf[3] = '\0'; (gdb) p c $21 = 1701869940 (gdb) p c&2 $22 = 0 (gdb) step 2785 r = s->strs + (c >> 2); (gdb) p s->strs $23 = 0x7ffff7ff240c "typeset" (gdb) p c>>2 $24 = 425467485 (gdb) p c $25 = 1701869940 (gdb) list 2780 buf[2] = (char) ((c >> 19) & 0xff); 2781 buf[3] = '\0'; 2782 r = dupstring(buf); 2783 dup = EC_NODUP; 2784 } else { 2785 r = s->strs + (c >> 2); 2786 } 2787 if (tokflag) 2788 *tokflag = (c & 1); 2789 (gdb) step 2787 if (tokflag) (gdb) p r $26 = 0x8000115b4269 <error: Cannot access memory at address 0x8000115b4269> -- Mikael Magnusson ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-14 0:13 ` Mikael Magnusson @ 2019-05-14 5:38 ` Bart Schaefer 0 siblings, 0 replies; 34+ messages in thread From: Bart Schaefer @ 2019-05-14 5:38 UTC (permalink / raw) To: Mikael Magnusson; +Cc: Oliver Kiddle, zsh-workers On Mon, May 13, 2019 at 5:13 PM Mikael Magnusson <mikachu@gmail.com> wrote: > > On 5/14/19, Oliver Kiddle <okiddle@yahoo.co.uk> wrote: > > On 10 May, Bart wrote: > >> > #3 Invalid read from *dupstring *in *string.c* > >> > POC folder: *03_dupstring_(string.c_39)* > > > > I've cut this one down to just: > > > > typeset Q= {X} > > Yeah it looks like some stuff is not exactly going right here, It's got something to do with handling it as a reserved word. BartMAC2014% disable -r typeset BartMAC2014% typeset Q= {X} typeset: not valid in this context: {X} ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) 2019-05-13 22:36 ` #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle 2019-05-14 0:13 ` Mikael Magnusson @ 2019-05-14 10:50 ` Peter Stephenson 1 sibling, 0 replies; 34+ messages in thread From: Peter Stephenson @ 2019-05-14 10:50 UTC (permalink / raw) To: zsh-workers On Tue, 2019-05-14 at 00:36 +0200, Oliver Kiddle wrote: > On 10 May, Bart wrote: > > > > > > > > #3 Invalid read from *dupstring *in *string.c* > > > POC folder: *03_dupstring_(string.c_39)* > > This gives exactly the same errors as #2, and then exits with > > > > [long ugly filename]:87: parse error near `}' > I've cut this one down to just: > > typeset Q= {X} > > That reliably seg faults for me. But that's about as far as I've > been able to get - I'm not especially familiar with zsh's parsing > code. Stepping through the parsing code when intypeset is set (with the optimiser turned off) made it fairly obvious where it was doing something it shouldn't, and the fix is to adapt code from below to this case... This is an obscure case we'd be very unlikely to pick up normally. The new parse case isn't actually useful and is bound to fail in the typeset, but the rational solution seems to be let the normal typeset code figure that out the same as if the Q= was missing (which I've also added a test for). pws diff --git a/Src/parse.c b/Src/parse.c index 22e553a16..27234497b 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -1899,6 +1899,14 @@ par_simple(int *cmplx, int nr) p += nrediradd; sr += nrediradd; } + else if (postassigns) + { + /* C.f. normal case below */ + postassigns++; + ecadd(WCB_ASSIGN(WC_ASSIGN_SCALAR, WC_ASSIGN_INC, 0)); + ecstr(toksave); + ecstr(""); /* TBD can possibly optimise out */ + } else { ecstr(toksave); diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst index ac86e0ad1..e7bf93794 100644 --- a/Test/B02typeset.ztst +++ b/Test/B02typeset.ztst @@ -1101,3 +1101,10 @@ >export zsh_exported_readonly_scalar=1 >readonly zsh_exported_readonly_array=( 2 ) >readonly zsh_exported_readonly_scalar=1 + + # The second case was buggy as it needs special handling in postassigns + (typeset {X}) + (typeset Q= {X}) +1:Regression test for {...} parsing in typeset +?(eval):typeset:2: not valid in this context: {X} +?(eval):typeset:3: not valid in this context: {X} ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer ` (3 preceding siblings ...) 2019-05-13 22:36 ` #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle @ 2019-05-14 16:38 ` Peter Stephenson 2019-05-14 20:30 ` Oliver Kiddle 5 siblings, 0 replies; 34+ messages in thread From: Peter Stephenson @ 2019-05-14 16:38 UTC (permalink / raw) To: zsh-workers On Fri, 2019-05-10 at 13:27 -0700, Bart Schaefer wrote: > On Fri, May 10, 2019 at 8:04 AM David Wells <bughunters@tenable.com> wrote: > > > > > > #1 Invalid read from *taddrstr *call in *text.c* > > POC folder: *01_taddstr_(text.c_148)* > This has literal NUL bytes embedded in the body of an if/then. Run > from an interactive shell, it gives: > > text.c:995: unknown word code in gettext2() > text.c:995: unknown word code in gettext2() > text.c:72: attempting to decrement tindent below zero > text.c:72: attempting to decrement tindent below zero > > and then (several seconds later) a crash. > > The following minimal subset of their test will put the shell into an > infinite loop, without (at least for as long as I was willing to wait) > crashing it: > > if true; then me > you || ! > : > fi So the best guess at the moment is the embedded NUL bytes are being misinterpreted by whatever causes the text to be handled wrongly, so they are only tangentially relevant? That would fit with what I'm seeing, which is the infinite loop is in gettext2(), before anything is executed. This function tries to decode wordcode set up by the parser, which is hard to debug because of the strong correlation between the two completely separate bits of code (and its own internal structure is a bit head-scratching, too). Might be interesting to perturb it until it just doesn't fail any more... The parsing phase seemed to finish normally, as far as I could see. pws ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer ` (4 preceding siblings ...) 2019-05-14 16:38 ` Zsh - Multiple DoS Vulnerabilities Peter Stephenson @ 2019-05-14 20:30 ` Oliver Kiddle 2019-05-15 16:50 ` Mikael Magnusson 2019-05-16 20:37 ` Peter Stephenson 5 siblings, 2 replies; 34+ messages in thread From: Oliver Kiddle @ 2019-05-14 20:30 UTC (permalink / raw) To: Zsh workers On 10 May, Bart wrote: > On Fri, May 10, 2019 at 8:04 AM David Wells <bughunters@tenable.com> wrote: > > > > #1 Invalid read from *taddrstr *call in *text.c* > > POC folder: *01_taddstr_(text.c_148)* > > and then (several seconds later) a crash. > > The following minimal subset of their test will put the shell into an > infinite loop, without (at least for as long as I was willing to wait) > crashing it: > > if true; then me > you || ! > : > fi I'm finding this one will crash on Linux but hang on FreeBSD. And not crash with true as the condition. A variety of things can be used in the condition. while .. do .. done can be used in place of if .. then .. fi, && or ||. The me > you part can be cut down to :. Try the following: if [[ m -eq y ]]; then : && ! : fi Where I had a crash, it was interpreting the wordcode in ecgetstr(). Where it does r = s->strs + (c >> 2), c had an infeasibly large value causing it to index well beyond the range of s->strs. I'd be inclined to suspect the problem comes earlier when parsing this into wordcode. Issues #2, #3 and #5 are not separate issues but slight variations all leading to the typeset followed by braces bug. So thanks to Peter, I think those are all now fixed leaving this (#1) as the only one outstanding. Oliver ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 20:30 ` Oliver Kiddle @ 2019-05-15 16:50 ` Mikael Magnusson 2019-05-16 20:37 ` Peter Stephenson 1 sibling, 0 replies; 34+ messages in thread From: Mikael Magnusson @ 2019-05-15 16:50 UTC (permalink / raw) To: Oliver Kiddle; +Cc: Zsh workers On 5/14/19, Oliver Kiddle <okiddle@yahoo.co.uk> wrote: > On 10 May, Bart wrote: >> On Fri, May 10, 2019 at 8:04 AM David Wells <bughunters@tenable.com> >> wrote: >> > >> > #1 Invalid read from *taddrstr *call in *text.c* >> > POC folder: *01_taddstr_(text.c_148)* >> >> and then (several seconds later) a crash. >> >> The following minimal subset of their test will put the shell into an >> infinite loop, without (at least for as long as I was willing to wait) >> crashing it: >> >> if true; then me > you || ! >> : >> fi > > I'm finding this one will crash on Linux but hang on FreeBSD. And not > crash with true as the condition. A variety of things can be used in the > condition. while .. do .. done can be used in place of if .. then .. fi, > && or ||. The me > you part can be cut down to :. Try the following: > > if [[ m -eq y ]]; then > : && ! > : > fi > > Where I had a crash, it was interpreting the wordcode in ecgetstr(). > Where it does r = s->strs + (c >> 2), c had an infeasibly large value > causing it to index well beyond the range of s->strs. I'd be inclined to > suspect the problem comes earlier when parsing this into wordcode. > > Issues #2, #3 and #5 are not separate issues but slight variations all > leading to the typeset followed by braces bug. So thanks to Peter, I think > those are all now fixed leaving this (#1) as the only one outstanding. Might it be worth adding some type of check to the ecgetstr() code, so we get a DPUTS instead of a crash if c>>2 is incredibly large? I'm not sure atm how this would be determined, or what typical values are, but I think two of these issues led to a crash here. We could also get arbitrary bytecode from a modified .zwc file although of course in that case you've already los tany security. Still would be nice to not crash from misparsing it though. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-14 20:30 ` Oliver Kiddle 2019-05-15 16:50 ` Mikael Magnusson @ 2019-05-16 20:37 ` Peter Stephenson 2019-05-17 13:41 ` Mikael Magnusson 1 sibling, 1 reply; 34+ messages in thread From: Peter Stephenson @ 2019-05-16 20:37 UTC (permalink / raw) To: zsh-workers On Tue, 2019-05-14 at 22:30 +0200, Oliver Kiddle wrote: > I'm finding this one will crash on Linux but hang on FreeBSD. And not > crash with true as the condition. A variety of things can be used in the > condition. while .. do .. done can be used in place of if .. then .. fi, > && or ||. The me > you part can be cut down to :. Try the following: > > if [[ m -eq y ]]; then > : && ! > : > fi > > Where I had a crash, it was interpreting the wordcode in ecgetstr(). > Where it does r = s->strs + (c >> 2), c had an infeasibly large value > causing it to index well beyond the range of s->strs. I'd be inclined to > suspect the problem comes earlier when parsing this into wordcode. I'm starting to wonder if this is an allocation rather than a parsing problem --- the parsing is OK but something goes wrong with the final pointer / afterwards / in building or copying the word code, so that gettext2() or the exec code ends up trying to interpret garbage at the end. pws ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-16 20:37 ` Peter Stephenson @ 2019-05-17 13:41 ` Mikael Magnusson 2019-05-17 13:51 ` Mikael Magnusson 0 siblings, 1 reply; 34+ messages in thread From: Mikael Magnusson @ 2019-05-17 13:41 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On 5/16/19, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > On Tue, 2019-05-14 at 22:30 +0200, Oliver Kiddle wrote: >> I'm finding this one will crash on Linux but hang on FreeBSD. And not >> crash with true as the condition. A variety of things can be used in the >> condition. while .. do .. done can be used in place of if .. then .. fi, >> && or ||. The me > you part can be cut down to :. Try the following: >> >> if [[ m -eq y ]]; then >> : && ! >> : >> fi >> >> Where I had a crash, it was interpreting the wordcode in ecgetstr(). >> Where it does r = s->strs + (c >> 2), c had an infeasibly large value >> causing it to index well beyond the range of s->strs. I'd be inclined to >> suspect the problem comes earlier when parsing this into wordcode. > > I'm starting to wonder if this is an allocation rather than a parsing > problem --- the parsing is OK but something goes wrong with the final > pointer / afterwards / in building or copying the word code, so > that gettext2() or the exec code ends up trying to interpret garbage at > the end. FWIW I ran this under valgrind, and the first invalid read is the one that causes the segfault, so no help there. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-17 13:41 ` Mikael Magnusson @ 2019-05-17 13:51 ` Mikael Magnusson 2019-05-17 14:28 ` Mikael Magnusson 2019-05-18 10:31 ` Oliver Kiddle 0 siblings, 2 replies; 34+ messages in thread From: Mikael Magnusson @ 2019-05-17 13:51 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On 5/17/19, Mikael Magnusson <mikachu@gmail.com> wrote: > On 5/16/19, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: >> On Tue, 2019-05-14 at 22:30 +0200, Oliver Kiddle wrote: >>> I'm finding this one will crash on Linux but hang on FreeBSD. And not >>> crash with true as the condition. A variety of things can be used in the >>> condition. while .. do .. done can be used in place of if .. then .. fi, >>> && or ||. The me > you part can be cut down to :. Try the following: >>> >>> if [[ m -eq y ]]; then >>> : && ! >>> : >>> fi >>> >>> Where I had a crash, it was interpreting the wordcode in ecgetstr(). >>> Where it does r = s->strs + (c >> 2), c had an infeasibly large value >>> causing it to index well beyond the range of s->strs. I'd be inclined to >>> suspect the problem comes earlier when parsing this into wordcode. >> >> I'm starting to wonder if this is an allocation rather than a parsing >> problem --- the parsing is OK but something goes wrong with the final >> pointer / afterwards / in building or copying the word code, so >> that gettext2() or the exec code ends up trying to interpret garbage at >> the end. > > FWIW I ran this under valgrind, and the first invalid read is the one > that causes the segfault, so no help there. Played with gdb reverse debugging a bit and found that at one point before the crash, we have this somewhat incorrect string built up: (gdb) p tptr-48 $28 = 0x6e7560 <jbuf> "if [[ m -eq y ]]; then; : && ! :; select G\305\305 in " -- Mikael Magnusson ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-17 13:51 ` Mikael Magnusson @ 2019-05-17 14:28 ` Mikael Magnusson 2019-05-18 10:31 ` Oliver Kiddle 1 sibling, 0 replies; 34+ messages in thread From: Mikael Magnusson @ 2019-05-17 14:28 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On 5/17/19, Mikael Magnusson <mikachu@gmail.com> wrote: > On 5/17/19, Mikael Magnusson <mikachu@gmail.com> wrote: >> On 5/16/19, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: >>> On Tue, 2019-05-14 at 22:30 +0200, Oliver Kiddle wrote: >>>> I'm finding this one will crash on Linux but hang on FreeBSD. And not >>>> crash with true as the condition. A variety of things can be used in >>>> the >>>> condition. while .. do .. done can be used in place of if .. then .. >>>> fi, >>>> && or ||. The me > you part can be cut down to :. Try the following: >>>> >>>> if [[ m -eq y ]]; then >>>> : && ! >>>> : >>>> fi >>>> >>>> Where I had a crash, it was interpreting the wordcode in ecgetstr(). >>>> Where it does r = s->strs + (c >> 2), c had an infeasibly large value >>>> causing it to index well beyond the range of s->strs. I'd be inclined >>>> to >>>> suspect the problem comes earlier when parsing this into wordcode. >>> >>> I'm starting to wonder if this is an allocation rather than a parsing >>> problem --- the parsing is OK but something goes wrong with the final >>> pointer / afterwards / in building or copying the word code, so >>> that gettext2() or the exec code ends up trying to interpret garbage at >>> the end. >> >> FWIW I ran this under valgrind, and the first invalid read is the one >> that causes the segfault, so no help there. > > Played with gdb reverse debugging a bit and found that at one point > before the crash, we have this somewhat incorrect string built up: > (gdb) p tptr-48 > $28 = 0x6e7560 <jbuf> "if [[ m -eq y ]]; then; : && ! :; select G\305\305 in > " If I save the above code in a file, named crash.zsh and run zsh -fc 'source crash.zsh' then it will crash. If I run zcompile on it, and then run the same command, I instead get the infinite loop in text.c: 420 if (stack) { (gdb) 421 if (!(s = tstack)) (gdb) 423 if (s->pop) { (gdb) 428 code = s->code; (gdb) 429 stack = 0; (gdb) 434 switch (wc_code(code)) { (gdb) 458 if (!s) { (gdb) 468 if (!(stack = (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END))) { (gdb) 479 if (stack < 1 && (WC_SUBLIST_FLAGS(s->code) & WC_SUBLIST_SIMPLE)) (gdb) 481 break; (gdb) 420 if (stack) { -- Mikael Magnusson ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-17 13:51 ` Mikael Magnusson 2019-05-17 14:28 ` Mikael Magnusson @ 2019-05-18 10:31 ` Oliver Kiddle 2019-05-21 14:43 ` Oliver Kiddle 1 sibling, 1 reply; 34+ messages in thread From: Oliver Kiddle @ 2019-05-18 10:31 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 3270 bytes --] Mikael Magnusson wrote: > Played with gdb reverse debugging a bit and found that at one point > before the crash, we have this somewhat incorrect string built up: > (gdb) p tptr-48 > $28 = 0x6e7560 <jbuf> "if [[ m -eq y ]]; then; : && ! :; select G\305\305 in " The point at which the code starts to go wrong is these lines in text.c: 450 s->code = *state->pc++; 451 s->pop = (WC_LIST_TYPE(s->code) & Z_END); code, which is what was popped from the stack is WC_LIST(type=SYNC, skip=0) - Z_END is not set. A comment in parse.c states: * - if not (type & Z_END), followed by next WC_LIST s->code is WC_END not WC_LIST. It doesn't seem valid to check WC_LIST_TYPE for that. We now iterate back round the loop picking up the garbage WC_SELECT(type=pparam, skip=27625473) value for the next code. That explains the select text that Mikael mentions. But I still have little idea what has led to the code doing this. After briefly trying to manually decode wordcode values, I hacked together a gdb pretty printer for them (attached) which makes it rather easier. Anyway the full wordcode looks like the following. I've manually tried to substitute codes corresponding to strings and indented for some of the skips but am not certain: especially the first string which might be a redirection code. List(type=SYNC|END, skip=0), SubList(type=END, skip=19), Pipe(type=end, line=4), If(type=head, skip=17), If(type=if, skip=16), List(type=SYNC|END|SIMPLE, skip=4), string Cond(-eq, skip=0), string string List(type=SYNC, skip=0), SubList(type=AND, skip=3), Pipe(type=end, line=5), Simple(argc=1), string SubList(type=END, flags=NOT, skip=0), List(type=SYNC|END, skip=0), SubList(type=END, skip=3), Pipe(type=end, line=6), Simple(argc=1), string End Without delving further into this, I'm somewhat unsure as to the meaning of the END types on lists and how start/end matching works along with the stack used in gettext2(). Maybe someone else knows it better? To enable the gdb pretty printer, just dump the file in the current directory and at the gdb prompt, type: python execfile("wordcode.py") To confirm, this worked: info pretty-printer To get line based output it can also be useful to do: set print array on And to watch the parse stage: watch *ecbuf@22 Note that it can't discern the placeholders for strings and I've not tested it exhaustively so there may be errors. There may be nicer ways to handle enabling the pretty-printer; you may have seen this if you've ever enabled to C++ STL pretty printers. Would it make sense to include things like this somewhere in the git repository? It is essentially a duplicate of C code so might bitrot relative to it. Also, I'm not especially fluent in Python, so it could perhaps be better. But it can be useful. I also have a gdb macro for the zle undo stack. Another aside, playing around with bit flags reminded me of a zsh annoyance: printing a negative number in binary gives you a minus sign followed by the positive representation of it rather than the two's complement form. Does it make sense to allow some form of unsigned output besides printf (which doesn't do binary)? And perhaps a Java style >>> operator. Oliver [-- Attachment #2: wordcode.py --] [-- Type: text/plain, Size: 5091 bytes --] # Gdb pretty printer for zsh wordcode values. # python execfile("wordcode.py") # set print array on # print *ecbuf@22 # note that you'll get garbage values for codes that reference strings from operator import itemgetter class WordcodePrinter: WC_END = 0 WC_LIST = 1 WC_SUBLIST = 2 WC_PIPE = 3 WC_REDIR = 4 WC_ASSIGN = 5 WC_SIMPLE = 6 WC_TYPESET = 7 WC_SUBSH = 8 WC_CURSH = 9 WC_TIMED = 10 WC_FUNCDEF = 11 WC_FOR = 12 WC_SELECT = 13 WC_WHILE = 14 WC_REPEAT = 15 WC_CASE = 16 WC_IF = 17 WC_COND = 18 WC_ARITH = 19 WC_AUTOFN = 20 WC_TRY = 21 WC_CODEBITS = 5 Z_END = (1<<4) Z_SIMPLE = (1<<5) WC_LIST_FREE = (6) NAME = [ "End", "List", "SubList", "Pipe", "Redir", "Assign", "Simple", "Typeset", "Subsh", "Cursh", "Timed", "Funcdef", "For", "Select", "While", "Repeat", "Case", "If", "Cond", "Arith", "Autofn", "Try" ] def __init__(self, val): self.val = val def wc_code(self, code): return int(code) & ((1 << self.WC_CODEBITS) - 1) def wc_data(self, code): return int(code) >> self.WC_CODEBITS def wc_list_type(self, data): return '|'.join(map(itemgetter(1), filter(lambda (i,s): data & (1<<i), enumerate(["TIMED", "SYNC", "ASYNC", "DISOWN", "END", "SIMPLE"])))) def wc_sublist_type(self, data): return [ "END", "AND", "OR" ][data & 3] def wc_redir_type(self, data): return [">", ">|", ">>", ">>|", "&>", ">&|", ">>&", ">>&|", "<>", "<", "<<", "<<-", "<<<", "<&n", ">&n", ">&-, <&-", "< <(...)", "> >(...)" ][data & 0x1f] def wc_sublist_flags(self, data): return '|'.join(map(itemgetter(1), filter(lambda (i,s): data & (1<<i), enumerate(["COPROC", "NOT", "SIMPLE" ], start=2)))) def wc_if_type(self, data): return [ "head", "if", "elif", "else" ][data & 3] def wc_for_type(self, data): return [ "pparam", "list", "cond" ][data & 3] def wc_case_type(self, data): return [ "head", "or", "and", "testand" ][data & 7] def wc_cond_type(self, data): return [ "!", "&&", "||", "=", "==", "!=", "<", ">", "-nt", "-ot", "-ef", "-eq", "-ne", "-lt", "-gt", "-le", "-ge", "=~", "MOD", "MOD(infix)" ][data & 127] def to_string(self): try: code = self.wc_code(self.val) data = self.wc_data(self.val) name = self.NAME[code] if (code == self.WC_LIST): name += '(type={}, skip={})'.format(self.wc_list_type(data), data >> 6) elif (code == self.WC_SUBLIST and data & 0x1c): name += '(type={}, flags={}, skip={})'.format( self.wc_sublist_type(data), self.wc_sublist_flags(data), data >> 5) elif (code == self.WC_SUBLIST): name += '(type={}, skip={})'.format(self.wc_sublist_type(data), data >> 5) elif (code == self.WC_REDIR): name += '(type={}{})'.format(self.wc_redir_type(data), ", varid=true" if (data & 0x20) else "") elif (code == self.WC_ASSIGN): if data & 2: name += "+=" name += "(array[{}])".format(data >> 2) if data & 1 else "(scalar)" elif (code == self.WC_SUBSH or code == self.WC_CURSH or code == self.WC_REPEAT or code == self.WC_TRY or code == self.WC_FUNCDEF): name += '(skip={})'.format(data) elif (code == self.WC_TIMED): name += '(type={})'.format("pipe" if data else "empty") elif (code == self.WC_PIPE): name += '(type={}, line={})'.format( "mid" if (data & 1) else "end", (data >> 1)) elif (code == self.WC_FOR): name += '(type={}, skip={})'.format(self.wc_for_type(data), data >> 2) elif (code == self.WC_SELECT): name += '(type={}, skip={})'.format( "pparam" if (data & 1) else "list", (data >> 1)) elif (code == self.WC_WHILE): if data & 1: name += '/until' name += '(skip={})'.format(data >> 1) elif (code == self.WC_CASE): name += '(type={}, skip={})'.format(self.wc_case_type(data), data >> 3) elif (code == self.WC_SIMPLE or code == self.WC_TYPESET): name += '(argc={})'.format(data) elif (code == self.WC_IF): name += '(type={}, skip={})'.format(self.wc_if_type(data), data >> 2) elif (code == self.WC_COND): name += '({}, skip={})'.format(self.wc_cond_type(data), data >> 7) except IndexError: name = int(code) # any error likely means it isn't a wordcode return name def zsh(val): if str(val.type) == 'wordcode': return WordcodePrinter(val) return None gdb.pretty_printers.append(zsh) ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Zsh - Multiple DoS Vulnerabilities 2019-05-18 10:31 ` Oliver Kiddle @ 2019-05-21 14:43 ` Oliver Kiddle [not found] ` <CGME20190521154256eucas1p1f0816d2467abd8bf4a0c31058af2983a@eucas1p1.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Oliver Kiddle @ 2019-05-21 14:43 UTC (permalink / raw) To: zsh-workers The following patch is one approach to fixing the last of these bugs. There may be a cleaner approach relying on the WC_SUBLIST_END tags, probably involving removing this whole block which is looking ahead to the next wordcode rather than leaving it for the next iteration of the big loop. But that would be a much bigger change with a greater chance of breaking things. The choice of "!" or "! " is a bit tricky and there may be some odd cases remaining wherever ! is used without something to negate. The code in exec.c that checks WC_SUBLIST_SKIP on the next code before knowing that it is a sublist is harmless because it only assigns the result to the next variable for later use. It still feels somewhat impure to my taste but I've left it. The patch also adds tests. Oliver diff --git a/Src/text.c b/Src/text.c index 3658b1bc6..a4191bf1a 100644 --- a/Src/text.c +++ b/Src/text.c @@ -470,8 +470,13 @@ gettext2(Estate state) " || " : " && "); s->code = *state->pc++; s->pop = (WC_SUBLIST_TYPE(s->code) == WC_SUBLIST_END); - if (WC_SUBLIST_FLAGS(s->code) & WC_SUBLIST_NOT) - taddstr("! "); + if (WC_SUBLIST_FLAGS(s->code) & WC_SUBLIST_NOT) { + if (WC_SUBLIST_SKIP(s->code) == 0) + stack = 1; + taddstr((stack || (!(WC_SUBLIST_FLAGS(s->code) & + WC_SUBLIST_SIMPLE) && wc_code(*state->pc) != + WC_PIPE)) ? "!" : "! "); + } if (WC_SUBLIST_FLAGS(s->code) & WC_SUBLIST_COPROC) taddstr("coproc "); } diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst index 1ed3cb6b7..c8600d4cb 100644 --- a/Test/A01grammar.ztst +++ b/Test/A01grammar.ztst @@ -76,6 +76,39 @@ 0:Basic current shell list with error >false + fn() { : && ! ; : } + functions -x3 fn + fn +0:End of sublist containing ! with no command +>fn () { +> : && ! +> : +>} + + if [[ m -eq y ]]; then + : && ! + : + fi +0:! followed by no further commands + + fn() { ! {!} && ! (!) || ! {!} } + functions -x2 fn + fn +0:exclamation marks without following commands +>fn () { +> ! { +> ! +> } && ! ( +> ! +> ) || ! { +> ! +> } +>} + + ! | true +1:! followed by no command but by a pipe +?(eval):1: parse error near `|' + # # Tests for `Precommand Modifiers' # ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20190521154256eucas1p1f0816d2467abd8bf4a0c31058af2983a@eucas1p1.samsung.com>]
* Re: Zsh - Multiple DoS Vulnerabilities [not found] ` <CGME20190521154256eucas1p1f0816d2467abd8bf4a0c31058af2983a@eucas1p1.samsung.com> @ 2019-05-21 15:42 ` Peter Stephenson 0 siblings, 0 replies; 34+ messages in thread From: Peter Stephenson @ 2019-05-21 15:42 UTC (permalink / raw) To: zsh-workers On Tue, 2019-05-21 at 16:43 +0200, Oliver Kiddle wrote: > The following patch is one approach to fixing the last of these bugs. > > There may be a cleaner approach relying on the WC_SUBLIST_END tags, > probably involving removing this whole block which is looking ahead to > the next wordcode rather than leaving it for the next iteration of the > big loop. But that would be a much bigger change with a greater chance > of breaking things. OK, so this takes account of the fact that "!" on its own (no following command line) is allowed and just means negate the status. That certainly seems a reasonable way to go. I was wondering whether this actually shouldn't be a special case in the parser, but it's not obvious what to do there --- there actually is nothing following the "!" and pretending there is something isn't a great fix. Telling the wordcode handler it can work this way is probably a better idea. Cheers pws ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2019-06-04 7:35 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-10 15:03 Zsh - Multiple DoS Vulnerabilities David Wells 2019-05-10 16:37 ` Bart Schaefer 2019-05-12 16:21 ` Stephane Chazelas 2019-05-13 16:29 ` David Wells 2019-05-13 22:02 ` Bart Schaefer 2019-05-14 18:10 ` Stephane Chazelas 2019-05-14 21:24 ` Daniel Shahaf 2019-05-14 21:38 ` Bart Schaefer 2019-05-14 21:39 ` Daniel Shahaf 2019-05-14 22:25 ` Bart Schaefer 2019-05-15 10:48 ` Daniel Shahaf 2019-05-31 12:05 ` [PATCH] [doc] [repost] warnings about restricted shell (Was: Zsh - Multiple DoS Vulnerabilities) Stephane Chazelas 2019-06-03 9:35 ` Peter Stephenson 2019-06-04 2:39 ` dana 2019-06-04 7:34 ` dana 2019-05-10 20:27 ` Zsh - Multiple DoS Vulnerabilities Bart Schaefer 2019-05-11 1:45 ` #7 (typeset -Tp) (was Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle 2019-05-13 9:01 ` Peter Stephenson 2019-05-13 21:11 ` PATCH: #6 negative job id (Re: " Oliver Kiddle 2019-05-13 21:44 ` Zsh - Multiple DoS Vulnerabilities Oliver Kiddle 2019-05-13 22:36 ` #3 typeset and braces (Re: Zsh - Multiple DoS Vulnerabilities) Oliver Kiddle 2019-05-14 0:13 ` Mikael Magnusson 2019-05-14 5:38 ` Bart Schaefer 2019-05-14 10:50 ` Peter Stephenson 2019-05-14 16:38 ` Zsh - Multiple DoS Vulnerabilities Peter Stephenson 2019-05-14 20:30 ` Oliver Kiddle 2019-05-15 16:50 ` Mikael Magnusson 2019-05-16 20:37 ` Peter Stephenson 2019-05-17 13:41 ` Mikael Magnusson 2019-05-17 13:51 ` Mikael Magnusson 2019-05-17 14:28 ` Mikael Magnusson 2019-05-18 10:31 ` Oliver Kiddle 2019-05-21 14:43 ` Oliver Kiddle [not found] ` <CGME20190521154256eucas1p1f0816d2467abd8bf4a0c31058af2983a@eucas1p1.samsung.com> 2019-05-21 15:42 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).