* Bug in Dash's unquoting of backslashes within backquoted strings
@ 2020-05-21 20:06 Matt Whitlock
2020-05-21 21:38 ` Ron Yorston
0 siblings, 1 reply; 5+ messages in thread
From: Matt Whitlock @ 2020-05-21 20:06 UTC (permalink / raw)
To: dash
A minimal example:
: `: "\\\\
\\$(bug)"`
That's one command, containing a newline character, embedded in a
double-quoted string, embedded in a backquoted string. The command is an
invocation of the : (colon) built-in, passing arguments obtained by word
splitting the result of the backquoted command substitution.
The Open Group says: "Within the backquoted style of command substitution,
backslash shall retain its literal meaning, except when followed by: '$',
'`', or '\' (dollar sign, backquote, backslash)." Thus, each pair of
backslashes between the backquotes is to be reduced to a single backslash.
Thus, the subcommand to be executed is:
: "\\
\$(bug)"
This subcommand is an invocation of the : (colon) built-in command, passing
a single argument obtained by performing quote removal on the double-quoted
string. After quote removal, the resulting argument consists of these eight
characters: backslash, newline, dollar sign, open parenthesis, b, u, g,
close parenthesis.
If the above subcommand is entered directly at the Dash command line, all
is well. However, when it appears inside a backquoted subcommand (with the
backslash characters being appropriately escaped), such as given at the top
of this report, then Dash processes it incorrectly:
/bin/sh: 1: bug: not found
Dash apparently skips over the backslash immediately following the newline
embedded in the double-quoted string. Thus, Dash sees the dollar sign as
introducing a command substitution rather than as a literal character.
This does not happen if the backslashes preceding the embedded newline are
absent. It also does not happen if any other character, including linear
whitespace, is inserted immediately after the embedded newline.
Both Bash and Busybox Ash get it right. Dash is non-conformant.
This bug has potentially grave security implications since Dash is
interpreting a string literal as though it were a command and executing it.
If the word "bug" in my examples above had been "rm -rf /" instead, the
results would have been catastrophic.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug in Dash's unquoting of backslashes within backquoted strings
2020-05-21 20:06 Bug in Dash's unquoting of backslashes within backquoted strings Matt Whitlock
@ 2020-05-21 21:38 ` Ron Yorston
2020-05-21 21:57 ` Ron Yorston
2020-05-21 22:02 ` Bug in Dash's unquoting of backslashes within backquoted strings Harald van Dijk
0 siblings, 2 replies; 5+ messages in thread
From: Ron Yorston @ 2020-05-21 21:38 UTC (permalink / raw)
To: dash, dash
Matt Whitlock wrote:
>A minimal example:
>
>: `: "\\\\
>\\$(bug)"`
>However, when it appears inside a backquoted subcommand (with the
>backslash characters being appropriately escaped), such as given at the top
>of this report, then Dash processes it incorrectly:
>
>/bin/sh: 1: bug: not found
This seems to have been introduced by commit 6bbc71d (parser: use
pgetc_eatbnl() in more places). Reverting the following part of the
commit makes the problem go away:
case '\\':
- if ((pc = pgetc()) == '\n') {
- nlprompt();
- /*
- * If eating a newline, avoid putting
- * the newline into the new character
- * stream (via the STPUTC after the
- * switch).
- */
- continue;
- }
+ pc = pgetc_eatbnl();
Ron
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug in Dash's unquoting of backslashes within backquoted strings
2020-05-21 21:38 ` Ron Yorston
@ 2020-05-21 21:57 ` Ron Yorston
2020-05-26 13:19 ` parser: Fix double-backslash nl in old-style command sub Herbert Xu
2020-05-21 22:02 ` Bug in Dash's unquoting of backslashes within backquoted strings Harald van Dijk
1 sibling, 1 reply; 5+ messages in thread
From: Ron Yorston @ 2020-05-21 21:57 UTC (permalink / raw)
To: dash, dash
Ron Yorston wrote:
>This seems to have been introduced by commit 6bbc71d (parser: use
>pgetc_eatbnl() in more places). Reverting the following part of the
>commit makes the problem go away:
>
> case '\\':
>- if ((pc = pgetc()) == '\n') {
>- nlprompt();
>- /*
>- * If eating a newline, avoid putting
>- * the newline into the new character
>- * stream (via the STPUTC after the
>- * switch).
>- */
>- continue;
>- }
>+ pc = pgetc_eatbnl();
Alternatively I see that BusyBox ash did this:
case '\\':
- pc = pgetc();
- if (pc == '\n') {
- nlprompt();
- /*
- * If eating a newline, avoid putting
- * the newline into the new character
- * stream (via the STPUTC after the
- * switch).
- */
- continue;
- }
+ pc = pgetc(); /* or pgetc_eatbnl()? why (example)? */
Ron
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Bug in Dash's unquoting of backslashes within backquoted strings
2020-05-21 21:38 ` Ron Yorston
2020-05-21 21:57 ` Ron Yorston
@ 2020-05-21 22:02 ` Harald van Dijk
1 sibling, 0 replies; 5+ messages in thread
From: Harald van Dijk @ 2020-05-21 22:02 UTC (permalink / raw)
To: Ron Yorston, dash, dash
On 21/05/2020 22:38, Ron Yorston wrote:
> Matt Whitlock wrote:
>> A minimal example:
>>
>> : `: "\\\\
>> \\$(bug)"`
>
>> However, when it appears inside a backquoted subcommand (with the
>> backslash characters being appropriately escaped), such as given at the top
>> of this report, then Dash processes it incorrectly:
>>
>> /bin/sh: 1: bug: not found
>
> This seems to have been introduced by commit 6bbc71d (parser: use
> pgetc_eatbnl() in more places). Reverting the following part of the
> commit makes the problem go away:
Yes, that commit (a patch I had submitted) was buggy and the part that
you suggest reverting should be.
Cheers,
Harald van Dijk
^ permalink raw reply [flat|nested] 5+ messages in thread
* parser: Fix double-backslash nl in old-style command sub
2020-05-21 21:57 ` Ron Yorston
@ 2020-05-26 13:19 ` Herbert Xu
0 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2020-05-26 13:19 UTC (permalink / raw)
To: Ron Yorston; +Cc: dash, dash
Ron Yorston <rmy@frippery.org> wrote:
>
> Alternatively I see that BusyBox ash did this:
>
> case '\\':
> - pc = pgetc();
> - if (pc == '\n') {
> - nlprompt();
> - /*
> - * If eating a newline, avoid putting
> - * the newline into the new character
> - * stream (via the STPUTC after the
> - * switch).
> - */
> - continue;
> - }
> + pc = pgetc(); /* or pgetc_eatbnl()? why (example)? */
Yes this is the correct fix.
---8<---
When handling backslashes within an old-style command substitution,
we should not call pgetc_eatbnl because that would treat the next
backslash character as another escape character if it was then
followed by a new-line.
This patch fixes it by calling pgetc.
Reported-by: Matt Whitlock <dash@mattwhitlock.name>
Fixes: 6bbc71d84bea ("parser: use pgetc_eatbnl() in more places")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/src/parser.c b/src/parser.c
index 3131045..03c103b 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -1393,7 +1393,7 @@ parsebackq: {
goto done;
case '\\':
- pc = pgetc_eatbnl();
+ pc = pgetc();
if (pc != '\\' && pc != '`' && pc != '$'
&& (!synstack->dblquote || pc != '"'))
STPUTC('\\', pout);
--
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-26 13:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 20:06 Bug in Dash's unquoting of backslashes within backquoted strings Matt Whitlock
2020-05-21 21:38 ` Ron Yorston
2020-05-21 21:57 ` Ron Yorston
2020-05-26 13:19 ` parser: Fix double-backslash nl in old-style command sub Herbert Xu
2020-05-21 22:02 ` Bug in Dash's unquoting of backslashes within backquoted strings Harald van Dijk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.