| Message ID | 20221217190705.atfwacsgggtafzl2@tarta.nabijaczleweli.xyz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | alias: aliascmd: refuse uninvokable aliases | expand |
наб <nabijaczleweli@nabijaczleweli.xyz> wrote: > [-- text/plain, encoding quoted-printable, charset: us-ascii, 109 lines --] > > See standards quote within, but the fun bit is: > alias "a'b=c" "ls&id=cd"; alias > outputs > ls&id='cd' > a'b='c' > neither of which is What You Want, and also you can't invoke them > because you need to escape the quote/&/whatever, which disables > alias processing. Forbid the minimum broken set. > > For reference's sake, here's a test driver: > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > #include <sys/wait.h> > #include <unistd.h> > int main(int _, char ** argv) { > putenv("LC_ALL=C"); > char val[] = "alias 'aQb=echo a' && alias"; > unsigned char * vp = strchr(val, 'Q'); > for(unsigned i = 0; i <= 0xFF; ++i) { > *vp = i; > if(!vfork()) { > execl(argv[1], "sh", "-c", val, (char *)NULL); > _exit(-1); > } > int r; > wait(&r); > fprintf(stderr, "%02x: %d\n", i, WEXITSTATUS(r)); > } > } > > zsh refuses nothing > dash refuses 09 0a 20 22 24 26 27 28 29 3b 3c 3e 5c 60 7c > bash refuses dash + 2f > mksh refuses bash + 23 + 2a + 3f + 5e + <20 + >7c > ksh refuses bash + 2a + 3f + 5b + 7b + 7d > > Fixes: https://bugs.debian.org/758542 The main objective of dash is to try to be minimal, so I'm not going to take this patch. Thanks,
Hi! On Sun, Dec 18, 2022 at 11:04:19AM +0800, Herbert Xu wrote: > The main objective of dash is to try to be minimal, so I'm not > going to take this patch. In that case, how's about the scissor-patch below that fixes the main questionable part of this (these dubious aliases executed as code when re-evaled, which POSIX says should be fine)? It's roughly what zsh does. -- >8 -- Subject: [PATCH] alias: printalias: quote the name, too This ensures even something like alias 'a|b|c=d' is output by alias as 'a|b|c'='d' instead of a|b|c='d' which is both "suitable for reinput to the shell" per POSIX and doesn't execute the aliases as code. --- src/alias.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/alias.c b/src/alias.c index daeacbb..1375cdd 100644 --- a/src/alias.c +++ b/src/alias.c @@ -197,7 +197,7 @@ freealias(struct alias *ap) { void printalias(const struct alias *ap) { - out1fmt("%s=%s\n", ap->name, single_quote(ap->val)); + out1fmt("%s=%s\n", single_quote(ap->name), single_quote(ap->val)); } STATIC struct alias **
On Sun, Dec 18, 2022 at 04:34:56AM +0100, наб wrote: > > In that case, how's about the scissor-patch below that fixes the main > questionable part of this (these dubious aliases executed as code when > re-evaled, which POSIX says should be fine)? It's roughly what zsh does. Yes this looks simple enough. I'll look into it. Thanks,
наб <nabijaczleweli@nabijaczleweli.xyz> wrote: > > -- >8 -- > Subject: [PATCH] alias: printalias: quote the name, too Please resend this with a new Subject line as otherwise it doesn't get tracked properly by patchwork. Thanks,
On 17/12/2022 19:07, наб wrote: > See standards quote within, but the fun bit is: > alias "a'b=c" "ls&id=cd"; alias > outputs > ls&id='cd' > a'b='c' > neither of which is What You Want, and also you can't invoke them > because you need to escape the quote/&/whatever, which disables > alias processing. Forbid the minimum broken set. I didn't realise this at the time, and since it's not getting merged it's not very important, but for completeness: this patch also rejects some aliases that are invokable: alias $=echo $ hello world This is because whether $ is special depends on the character that follows. Cheers, Harald van Dijk
diff --git a/src/alias.c b/src/alias.c index daeacbb..81de975 100644 --- a/src/alias.c +++ b/src/alias.c @@ -32,6 +32,7 @@ * SUCH DAMAGE. */ +#include <stdbool.h> #include <stdlib.h> #include "shell.h" #include "input.h" @@ -46,10 +47,38 @@ struct alias *atab[ATABSIZE]; +STATIC bool validalias(const char *); STATIC void setalias(const char *, const char *); STATIC struct alias *freealias(struct alias *); STATIC struct alias **__lookupalias(const char *); +/* + * POSIX Issue 7, XCU, 2.3.1 Alias Substitution: + * the command name word of a simple command shall be examined + * to determine whether it is an unquoted, valid alias name + * POSIX Issue 7, XCU, 2.2 Quoting: + * The various quoting mechanisms are the escape character, + * single-quotes, and double-quotes. + * + * Trivially, it's impossible to invoke an alias with whitespace inside + * (which has to be quoted, and therefore won't be an alias), + * with a backslash, or with quotes (likewise). + * The same applies to $, &, ), (, ;, >, <, `, and |. + * + * Additionally, rejecting quotes and other garbage means that we prevent + * alias "a'b=c" "ls&id=cd" + * being output as + * a'b='c' + * ls&id='cd' + * which explodes and/or executes code when it's evaled back. + */ +STATIC +bool +validalias(const char *name) +{ + return !strpbrk(name, "\t\n \\\"'$&)(;><`|"); +} + STATIC void setalias(const char *name, const char *val) @@ -151,7 +180,11 @@ aliascmd(int argc, char **argv) printalias(ap); } else { *v++ = '\0'; - setalias(n, v); + if (!validalias(n)) { + outfmt(out2, "%s: %s: invalid name\n", "alias", n); + ret = 1; + } else + setalias(n, v); } }
See standards quote within, but the fun bit is: alias "a'b=c" "ls&id=cd"; alias outputs ls&id='cd' a'b='c' neither of which is What You Want, and also you can't invoke them because you need to escape the quote/&/whatever, which disables alias processing. Forbid the minimum broken set. For reference's sake, here's a test driver: #include <stdio.h> #include <stdlib.h> #include <string.h> #include <sys/wait.h> #include <unistd.h> int main(int _, char ** argv) { putenv("LC_ALL=C"); char val[] = "alias 'aQb=echo a' && alias"; unsigned char * vp = strchr(val, 'Q'); for(unsigned i = 0; i <= 0xFF; ++i) { *vp = i; if(!vfork()) { execl(argv[1], "sh", "-c", val, (char *)NULL); _exit(-1); } int r; wait(&r); fprintf(stderr, "%02x: %d\n", i, WEXITSTATUS(r)); } } zsh refuses nothing dash refuses 09 0a 20 22 24 26 27 28 29 3b 3c 3e 5c 60 7c bash refuses dash + 2f mksh refuses bash + 23 + 2a + 3f + 5e + <20 + >7c ksh refuses bash + 2a + 3f + 5b + 7b + 7d Fixes: https://bugs.debian.org/758542 --- src/alias.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)