- use the hook provided by mmdebstrap instead of cooking up our own
thing, thus preventing code duplication
- this also copies over apt pinning rules which were missing before
On Tue, Dec 06, 2022 at 10:15:03AM +0000, Harald van Dijk wrote:
>
> There is a long-standing bug that may or may not be harder to fix if this
> patch goes in, depending on how you want to fix it. Here's a script that
> already fails on current dash.
>
> f() {
> if ! return 0
> then :
> fi
> }
> f
>
> This should return 0, and does return 0 in bash and ksh (and almost all
> shells), but returns 1 in dash.
>
> There are a few possible ways of fixing it. Some of them rely on continuing
> to conditionally set exitstatus.
This can be fixed simply by testing evalskip prior to flipping the
status.
Reported-by: Harald van Dijk <harald@gigawatt.nl>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Harald van Dijk <harald@gigawatt.nl> wrote:
> On 21/11/2022 13:08, Harald van Dijk wrote:
>> On 21/11/2022 02:38, Christoph Anton Mitterer wrote:
>>> reject_filtered_cmd()
>>> {
>>> reject_and_die "disallowed command${restrict_path_list:+
>>> (restrict-path: \"${restrict_path_list//|/\", \"}\")}"
>>> }
>>>
>>> reject_filtered_cmd
>>[...]
>> This should either result in the ${...//...} being skipped, or the "Bad
>> substitution" error. Currently, what happens instead is it attempts, but
>> fails, to skip the ${...//...}.
>
> The reason it fails is because the word is cut off.
>
> Variable substitutions are encoded as a CTLVAR special character,
> followed by a byte indicating the type of substitution, followed by the
> rest of the substitution data. The type of substitution is the VSNORMAL,
> VSMINUS, etc. seen in parser.h. An invalid substitution is encoded as a
> value of 0.
>
> When we define a function, we clone the function body in order to
> preserve it. Cloning the function body is done by cloning each node.
> Cloning a "word" node (NARG) involves copying the characters that make
> up the word up to and including the terminating null byte.
>
> These two interact badly. The invalid substitution is seen as
> terminating the word, the rest of the word is not copied, but the
> expansion code does not have any way of seeing that anything got cut off
> and happily continues attempting to process the rest of the word.
>
> If dash decides to issue an error in this case, this is not a problem:
> the null byte is guaranteed to be copied, and if processing is
> guaranteed to stop if a null byte is encountered, everything works out.
>
> If dash decides to not issue an error in this case, the encoding of bad
> substitutions needs to change to a non-null byte. It appears that if we
> set the byte to VSNUL, the expansion logic is already able to handle it,
> but I have not tested this extensively.
Thanks for the analysis Harald!
This patch does basically what you've described except it uses a new
bit to avoid any confusion with a genuine VSNUL.
Fixes: 3df3edd13389 ("[PARSER] Report substition errors at...")
Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> 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>
On Mon, Jun 20, 2022 at 02:27:10PM -0400, Alex Gorinson wrote:
> Due to a logic error in the ifsbreakup function in expand.c if a
> heredoc and normal command is run one after the other by means of a
> semi-colon, when the second command drops into ifsbreakup the command
> will be evaluated with the ifslastp/ifsfirst struct that was set when
> the here doc was evaluated. This results in a buffer over-read that
> can leak the program's heap, stack, and arena addresses which can be
> used to beat ASLR.
>
> Steps to Reproduce:
> First bug:
> cmd args: ~/exampleDir/example> dash
> $ M='AAAAAAAAAAAAAAAAA' <note: 17 A's>
> $ q00(){
> $ <<000;echo
> $ ${D?$M$M$M$M$M$M} <note: 6 $M's>
> $ 000
> $ }
> $ q00 <note: After the q00 is typed in, the leak
> should be echo'd out; this works with ash, busybox ash, and dash and
> with all option args.>
>
> Patch:
> Adding the following to expand.c will fix both bugs in one go.
> (Thank you to Harald van Dijk and Michael Greenberg for doing the
> heavy lifting for this patch!)
> ==========================
> --- a/src/expand.c
> +++ b/src/expand.c
> @@ -859,6 +859,7 @@
> if (discard)
> return -1;
>
> +ifsfree();
> sh_error("Bad substitution");
> }
>
> @@ -1739,6 +1740,7 @@
> } else
> msg = umsg;
> }
> +ifsfree();
> sh_error("%.*s: %s%s", end - var - 1, var, msg, tail);
> }
> ==========================
Thanks for the report!
I think it's better to add the ifsfree() call to the exception
handling path as other sh_error calls may trigger this too.
Reported-by: Alex Gorinson <algore3698@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This patch forces ^ to be a literal when we use fnmatch.
In order to allow for the extra space to quote the caret, the
function _rmescapes will allocate up to twice the memory if the
flag RMESCAPE_GLOB is set.
Fixes: 7638476c18f2 ("shell: Enable fnmatch/glob by default")
Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
Suggested-by: Harald van Dijk <harald@gigawatt.nl>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Commit 17db43b5841504b694203952fb0e82246c06a97f (input: Allow two
consecutive calls to pungetc) ensures that EOF is handled like any
other character with respect to unget. As a result it's possible
to remove the special case for unget of EOF in preadbuffer.
Signed-off-by: Ron Yorston <rmy@frippery.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
- We use a variable because we reordered the code in the last commit
such that dfile now always exists, so we check beforehand
- We also check -L because -e dereferences the symlink
This is to allow downgrades to version 0.5.11+git20210903+057cd650a4ed-3 and
earlier which do not use the update-shells trigger of debianutils to manage
/etc/shells.
update-shells will be called at the end of the downgrade because a trigger of
debianutils on the removal of /usr/share/debianutils/shells.d/dash will be
activated when downgrading to a version of dash that doesn't ship that file.
But a non-existing /usr/share/debianutils/shells.d/dash tells update-shells
to remove dash from /etc/shells.
To prevent this from happening, remove /usr/share/debianutils/shells.d/dash
which will be removed by the downgrade anyways, then run update-shells which
will remove dash from /etc/shells and then add it again by running add-shell.
Subsequent calls to update-shells in the debianutils trigger will now not
remove dash from /etc/shells anymore because the update-shells call in this
script updated /var/lib/shells.state with the information that it doesn't
manage dash via update-shells anymore.
