Fwd: Bug#1017531: dash: for/while/if suppress errors from redirections with -e

Fwd: Bug#1017531: dash: for/while/if suppress errors from redirections with -e

[наб]

[-- Attachment #1: Type: text/plain, Size: 3796 bytes --]

Hi!

Thought I'd forward this upstream, considering I already tested on
current git, and it's pretty grave.

https://bugs.debian.org/1017531, trimmed down a bit, follows:

----- Forwarded message from наб <nabijaczleweli@nabijaczleweli.xyz> -----

Subject: Bug#1017531: dash: for/while/if suppress errors from redirections
 with -e, POSIX violation

Dear Maintainer,

On current git (057cd650a4edd5856213d431a974ff35c6594489) and 0.5.11.5
the following holds:
-- >8 --
$ ./src/dash -ec 'while :; do :; done < /ENOENT; echo uhoh'
./src/dash: 1: cannot open /ENOENT: No such file
uhoh
$ ./src/dash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
./src/dash: 1: cannot open /ENOENT: No such file
uhoh
$ ./src/dash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
./src/dash: 1: cannot open /ENOENT: No such file
uhoh
-- >8 --

The correct output, as demonstrated by bash, is:
-- >8 --
$ bash -ec 'while :; do :; done < /ENOENT; echo uhoh'
bash: line 1: /ENOENT: No such file or directory
$ bash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
bash: line 1: /ENOENT: No such file or directory
$ bash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
bash: line 1: /ENOENT: No such file or directory
-- >8 --

This is a POSIX violation, and quite a grave one at that:
set -e is oft[1] used to guard against precisely this type of error!

The same happens if set -e is executed.

All quotes POSIX.1, Issue 7, TC2:
sh, OPTIONS:
  > The -a, -b, -C, -e, -f, -m, -n, -o option, -u, -v, and -x options
  > are described as part of the set utility in Special Built-In
  > Utilities. 

set, DESCRIPTION, -e:
  > When this option is on, when any command fails (for any of the
  > reasons listed in Consequences of Shell Errors or by returning an
  > exit status greater than zero), the shell immediately shall exit, as
  > if by executing the exit special built-in utility with no arguments,
  > with the following exceptions:
  >
  > 1. The failure of any individual command in a multi-command pipeline
  >    shall not cause the shell to exit. Only the failure of the
  >    pipeline itself shall be considered.
  > 2. The -e setting shall be ignored when executing the compound list
  >    following the while, until, if, or elif reserved word, a pipeline
  >    beginning with the ! reserved word, or any command of an AND-OR
  >    list other than the last.
  > 3. If the exit status of a compound command other than a subshell
  >    command was the result of a failure while -e was being ignored,
  >    then -e shall not apply to this command.

XCU, 2.9.4: Shell Command Language, Shell Commands, Compound Commands:
The while Loop:
  > The format of the while loop is as follows:
  > 
  > while compound-list-1
  > do
  >   compound-list-2
  > done
(until is equivalent).
The if Conditional Construct: 
  > The format for the if construct is as follows:
  >
  > if compound-list
  > then
  >   compound-list
  > [elif compound-list
  > then
  >   compound-list] ...
  > [else
  >   compound-list]
  > fi

It follows, therefore, that
  * Exception 1. does not apply as there is no pipeline
  * Exception 2. does not apply, as the redirection does /not/ follow
    "while" or "if" directly and is /not/ part of the conditional
	compound-list
  * in the "for" case, there is no such provision, so this is likely not
    a confusion w.r.t. the conditional compound-lists
  * Exception 3. does not apply as -e was not being ignored while the
    compound commands were being executed (indeed, the compound commands
    do not run at all, as evidenced by the program terminating)

[1]: https://salsa.debian.org/glibc-team/glibc/-/merge_requests/6#note_329899
----- End forwarded message -----

Best,
наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Fwd: Bug#1017531: dash: for/while/if suppress errors from redirections with -e

[Michael Greenberg]

This looks like a serious bug... nice find!

I just checked my reference semantics shell (smoosh), and it fails this
test as well. Notably, smoosh passes the POSIX test suite---which means
that the POSIX folks are not properly testing for this behavior.

Also: what version of bash are you running? I suspect the fix to this in
bash is fairly recent.

$ bash --version
GNU bash, version 4.4.12(1)-release (x86_64-apple-darwin17.7.0)
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
$ bash -ec 'while :; do :; done < /ENOENT; echo uhoh'
bash: /ENOENT: No such file or directory
uhoh

$ /bin/bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin17)
Copyright (C) 2007 Free Software Foundation, Inc.
$ /bin/bash -ec 'while :; do :; done < /ENOENT; echo uhoh'
/bin/bash: /ENOENT: No such file or directory
uhoh

Cheers,
Michael

On 2022-08-18 at 01:44:12 PM, наб wrote:

> Hi!
>
> Thought I'd forward this upstream, considering I already tested on
> current git, and it's pretty grave.
>
> https://bugs.debian.org/1017531, trimmed down a bit, follows:
>
> ----- Forwarded message from наб <nabijaczleweli@nabijaczleweli.xyz> -----
>
> Subject: Bug#1017531: dash: for/while/if suppress errors from redirections
>  with -e, POSIX violation
>
> Dear Maintainer,
>
> On current git (057cd650a4edd5856213d431a974ff35c6594489) and 0.5.11.5
> the following holds:
> -- >8 --
> $ ./src/dash -ec 'while :; do :; done < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> $ ./src/dash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> $ ./src/dash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> -- >8 --
>
> The correct output, as demonstrated by bash, is:
> -- >8 --
> $ bash -ec 'while :; do :; done < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> $ bash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> $ bash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> -- >8 --
>
> This is a POSIX violation, and quite a grave one at that:
> set -e is oft[1] used to guard against precisely this type of error!
>
> The same happens if set -e is executed.
>
> All quotes POSIX.1, Issue 7, TC2:
> sh, OPTIONS:
>   > The -a, -b, -C, -e, -f, -m, -n, -o option, -u, -v, and -x options
>   > are described as part of the set utility in Special Built-In
>   > Utilities. 
>
> set, DESCRIPTION, -e:
>   > When this option is on, when any command fails (for any of the
>   > reasons listed in Consequences of Shell Errors or by returning an
>   > exit status greater than zero), the shell immediately shall exit, as
>   > if by executing the exit special built-in utility with no arguments,
>   > with the following exceptions:
>   >
>   > 1. The failure of any individual command in a multi-command pipeline
>   >    shall not cause the shell to exit. Only the failure of the
>   >    pipeline itself shall be considered.
>   > 2. The -e setting shall be ignored when executing the compound list
>   >    following the while, until, if, or elif reserved word, a pipeline
>   >    beginning with the ! reserved word, or any command of an AND-OR
>   >    list other than the last.
>   > 3. If the exit status of a compound command other than a subshell
>   >    command was the result of a failure while -e was being ignored,
>   >    then -e shall not apply to this command.
>
> XCU, 2.9.4: Shell Command Language, Shell Commands, Compound Commands:
> The while Loop:
>   > The format of the while loop is as follows:
>   > 
>   > while compound-list-1
>   > do
>   >   compound-list-2
>   > done
> (until is equivalent).
> The if Conditional Construct: 
>   > The format for the if construct is as follows:
>   >
>   > if compound-list
>   > then
>   >   compound-list
>   > [elif compound-list
>   > then
>   >   compound-list] ...
>   > [else
>   >   compound-list]
>   > fi
>
> It follows, therefore, that
>   * Exception 1. does not apply as there is no pipeline
>   * Exception 2. does not apply, as the redirection does /not/ follow
>     "while" or "if" directly and is /not/ part of the conditional
> 	compound-list
>   * in the "for" case, there is no such provision, so this is likely not
>     a confusion w.r.t. the conditional compound-lists
>   * Exception 3. does not apply as -e was not being ignored while the
>     compound commands were being executed (indeed, the compound commands
>     do not run at all, as evidenced by the program terminating)
>
> [1]: https://salsa.debian.org/glibc-team/glibc/-/merge_requests/6#note_329899
> ----- End forwarded message -----
>
> Best,
> наб

Re: Fwd: Bug#1017531: dash: for/while/if suppress errors from redirections with -e

[наб]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

On Thu, Aug 18, 2022 at 10:22:25AM -0400, Michael Greenberg wrote:
> Also: what version of bash are you running? I suspect the fix to this in
> bash is fairly recent.
5.1-* off Debian; a simple default-config build+./bash -ec ... check
across the distribution tarballs shows that 4.4.18 and 5.0 are broken
in the same way as dash (and like your transcripts show),
but 5.1 produces the correct output.

наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] eval: Check eflag after redirection error

[Herbert Xu]

наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
> 
> Thought I'd forward this upstream, considering I already tested on
> current git, and it's pretty grave.
> 
> https://bugs.debian.org/1017531, trimmed down a bit, follows:
> 
> ----- Forwarded message from наб <nabijaczleweli@nabijaczleweli.xyz> -----
> 
> Subject: Bug#1017531: dash: for/while/if suppress errors from redirections
> with -e, POSIX violation
> 
> Dear Maintainer,
> 
> On current git (057cd650a4edd5856213d431a974ff35c6594489) and 0.5.11.5
> the following holds:
> -- >8 --
> $ ./src/dash -ec 'while :; do :; done < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> $ ./src/dash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> $ ./src/dash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
> ./src/dash: 1: cannot open /ENOENT: No such file
> uhoh
> -- >8 --
> 
> The correct output, as demonstrated by bash, is:
> -- >8 --
> $ bash -ec 'while :; do :; done < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> $ bash -ec 'for _ in :; do :; done < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> $ bash -ec 'if :; then :; fi < /ENOENT; echo uhoh'
> bash: line 1: /ENOENT: No such file or directory
> -- >8 --
> 
> This is a POSIX violation, and quite a grave one at that:
> set -e is oft[1] used to guard against precisely this type of error!
> 
> The same happens if set -e is executed.
> 
> All quotes POSIX.1, Issue 7, TC2:
> sh, OPTIONS:
>  > The -a, -b, -C, -e, -f, -m, -n, -o option, -u, -v, and -x options
>  > are described as part of the set utility in Special Built-In
>  > Utilities. 
> 
> set, DESCRIPTION, -e:
>  > When this option is on, when any command fails (for any of the
>  > reasons listed in Consequences of Shell Errors or by returning an
>  > exit status greater than zero), the shell immediately shall exit, as
>  > if by executing the exit special built-in utility with no arguments,
>  > with the following exceptions:
>  >
>  > 1. The failure of any individual command in a multi-command pipeline
>  >    shall not cause the shell to exit. Only the failure of the
>  >    pipeline itself shall be considered.
>  > 2. The -e setting shall be ignored when executing the compound list
>  >    following the while, until, if, or elif reserved word, a pipeline
>  >    beginning with the ! reserved word, or any command of an AND-OR
>  >    list other than the last.
>  > 3. If the exit status of a compound command other than a subshell
>  >    command was the result of a failure while -e was being ignored,
>  >    then -e shall not apply to this command.
> 
> XCU, 2.9.4: Shell Command Language, Shell Commands, Compound Commands:
> The while Loop:
>  > The format of the while loop is as follows:
>  > 
>  > while compound-list-1
>  > do
>  >   compound-list-2
>  > done
> (until is equivalent).
> The if Conditional Construct: 
>  > The format for the if construct is as follows:
>  >
>  > if compound-list
>  > then
>  >   compound-list
>  > [elif compound-list
>  > then
>  >   compound-list] ...
>  > [else
>  >   compound-list]
>  > fi
> 
> It follows, therefore, that
>  * Exception 1. does not apply as there is no pipeline
>  * Exception 2. does not apply, as the redirection does /not/ follow
>    "while" or "if" directly and is /not/ part of the conditional
>        compound-list
>  * in the "for" case, there is no such provision, so this is likely not
>    a confusion w.r.t. the conditional compound-lists
>  * Exception 3. does not apply as -e was not being ignored while the
>    compound commands were being executed (indeed, the compound commands
>    do not run at all, as evidenced by the program terminating)
> 
> [1]: https://salsa.debian.org/glibc-team/glibc/-/merge_requests/6#note_329899
> ----- End forwarded message -----

Yes we should check the exit status after redirections.

Reported-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/eval.c b/src/eval.c
index eda251e..7aa5cc2 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -246,15 +246,18 @@ evaltree(union node *n, int flags)
 			lineno -= funcline - 1;
 		expredir(n->nredir.redirect);
 		pushredir(n->nredir.redirect);
-		status = redirectsafe(n->nredir.redirect, REDIR_PUSH) ?:
-			 evaltree(n->nredir.n, flags & EV_TESTED);
+		status = redirectsafe(n->nredir.redirect, REDIR_PUSH);
+		if (status)
+			checkexit = EV_TESTED;
+		else
+			status = evaltree(n->nredir.n, flags & EV_TESTED);
 		if (n->nredir.redirect)
 			popredir(0);
 		break;
 	case NCMD:
 		evalfn = evalcommand;
 checkexit:
-		checkexit = ~flags & EV_TESTED;
+		checkexit = EV_TESTED;
 		goto calleval;
 	case NFOR:
 		evalfn = evalfor;
@@ -316,7 +319,7 @@ calleval:
 out:
 	dotrap();
 
-	if (eflag && checkexit && status)
+	if (eflag && (~flags & checkexit) && status)
 		goto exexit;
 
 	if (flags & EV_EXIT) {
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt