Re: [patch] libc: wordexp support
Hello, On Tue, Apr 06, 2010 at 02:34:03PM -0600, Theo de Raadt wrote: > > On Tue, Apr 06, 2010 at 11:55:21AM -0400, Ted Unangst wrote: > > I'm sorry, but this is terrible. (Not your effort, which is > > appreciated, but the whole function.) I do not like the idea of > > adding a "be extra careful or you will introduce a backdoor" > > function to libc. > > No kidding. I agree with Ted; thanks for the effort but the code you > borrowed is bad, and it is was designed by people who the unabomber > should have gotten to first. Understood. Moving on. Matthew
Re: [patch] libc: wordexp support
> If I continue to have say in this, wordexp() will never be in > OpenBSD's libc. Perhaps if there is one open source project which > refuses to have it, coders will continue avoiding use of wordexp(). Other than we know by the example of strlc* that doesnt' work ;) Even though there are real reasons this is dumb, as opposed to ulrichisms.
Re: [patch] libc: wordexp support
> On Tue, Apr 6, 2010 at 1:53 AM, Matthew Haub > wrote: > > This patch adds support for wordexp(3) and wordfree(3) to libc. These > > functions conform to IEEE Std 1003.1-2001 (POSIX). The implementation > > comes from NetBSD and uses a shell builtin, "wordexp", to perform the > > expansion in line with the methods suggested in the specification[1]. > > > [1] http://www.opengroup.org/onlinepubs/9699919799/functions/wordexp.html > > "Therefore, the application shall ensure that words does not contain > an unquoted character or any of the unquoted shell special > characters '|' , '&' , ';' , '<' , '>' except in the context of > command substitution as specified in XCU Command Substitution . It > also shall not contain unquoted parentheses or braces, except in the > context of command or variable substitution. The application shall > ensure that every member of words which it expects to have expanded by > wordexp() does not contain an unquoted initial comment character. The > application shall also ensure that any words which it intends to be > ignored (because they begin or continue a comment) are deleted from > words." > > What a load of crap. I've done a bit of digging myself, during a long discussion we had about this. I find very little open source software using this API. Of those that I find which are using it, many are using it wrong. Some are exposing themselves to risk. Almost 10 years ago we found security holes in glob(), and they were pretty important because of what used the code. Yet, glob() is a lot simpler than what this is trying to do. wordexp() is diseased. > > +.Sh BUGS > > +Do not pass untrusted user data to > > +.Fn wordexp , > > +regardless of whether the > > +.Dv WRDE_NOCMD > > +flag is set. > > +The > > +.Fn wordexp > > +function attempts to detect input that would cause commands to be > > +executed before passing it to the shell > > +but it does not use the same parser so it may be fooled. > > I'm sorry, but this is terrible. (Not your effort, which is > appreciated, but the whole function.) I do not like the idea of > adding a "be extra careful or you will introduce a backdoor" function > to libc. No kidding. I agree with Ted; thanks for the effort but the code you borrowed is bad, and it is was designed by people who the unabomber should have gotten to first. > Also, a libc function that doesn't work in chroot? What use is that? I found a piece of code out there that as wrapping the calls to wordexp() like this: sigemptyset(&nset); sigaddset(&nset, SIGCHLD); sigprocmask(SIG_BLOCK, &nset, NULL); i = wordexp(name, &we, 0); sigprocmask(SIG_UNBLOCK, &nset, NULL); Any function which requires that does not belong in libc. If I continue to have say in this, wordexp() will never be in OpenBSD's libc. Perhaps if there is one open source project which refuses to have it, coders will continue avoiding use of wordexp().
Re: [patch] libc: wordexp support
I think we should stand up to crap and not ever impliment it. > On Tue, Apr 6, 2010 at 1:53 AM, Matthew Haub > wrote: > > This patch adds support for wordexp(3) and wordfree(3) to libc. These > > functions conform to IEEE Std 1003.1-2001 (POSIX). The implementation > > comes from NetBSD and uses a shell builtin, "wordexp", to perform the > > expansion in line with the methods suggested in the specification[1]. > > > [1] http://www.opengroup.org/onlinepubs/9699919799/functions/wordexp.html > > "Therefore, the application shall ensure that words does not contain > an unquoted character or any of the unquoted shell special > characters '|' , '&' , ';' , '<' , '>' except in the context of > command substitution as specified in XCU Command Substitution . It > also shall not contain unquoted parentheses or braces, except in the > context of command or variable substitution. The application shall > ensure that every member of words which it expects to have expanded by > wordexp() does not contain an unquoted initial comment character. The > application shall also ensure that any words which it intends to be > ignored (because they begin or continue a comment) are deleted from > words." > > What a load of crap. > > > +.Sh BUGS > > +Do not pass untrusted user data to > > +.Fn wordexp , > > +regardless of whether the > > +.Dv WRDE_NOCMD > > +flag is set. > > +The > > +.Fn wordexp > > +function attempts to detect input that would cause commands to be > > +executed before passing it to the shell > > +but it does not use the same parser so it may be fooled. > > I'm sorry, but this is terrible. (Not your effort, which is > appreciated, but the whole function.) I do not like the idea of > adding a "be extra careful or you will introduce a backdoor" function > to libc. > > Also, a libc function that doesn't work in chroot? What use is that?
Re: [patch] libc: wordexp support
On Tue, Apr 6, 2010 at 1:53 AM, Matthew Haub wrote: > This patch adds support for wordexp(3) and wordfree(3) to libc. These > functions conform to IEEE Std 1003.1-2001 (POSIX). The implementation > comes from NetBSD and uses a shell builtin, "wordexp", to perform the > expansion in line with the methods suggested in the specification[1]. > [1] http://www.opengroup.org/onlinepubs/9699919799/functions/wordexp.html "Therefore, the application shall ensure that words does not contain an unquoted character or any of the unquoted shell special characters '|' , '&' , ';' , '<' , '>' except in the context of command substitution as specified in XCU Command Substitution . It also shall not contain unquoted parentheses or braces, except in the context of command or variable substitution. The application shall ensure that every member of words which it expects to have expanded by wordexp() does not contain an unquoted initial comment character. The application shall also ensure that any words which it intends to be ignored (because they begin or continue a comment) are deleted from words." What a load of crap. > +.Sh BUGS > +Do not pass untrusted user data to > +.Fn wordexp , > +regardless of whether the > +.Dv WRDE_NOCMD > +flag is set. > +The > +.Fn wordexp > +function attempts to detect input that would cause commands to be > +executed before passing it to the shell > +but it does not use the same parser so it may be fooled. I'm sorry, but this is terrible. (Not your effort, which is appreciated, but the whole function.) I do not like the idea of adding a "be extra careful or you will introduce a backdoor" function to libc. Also, a libc function that doesn't work in chroot? What use is that?
[patch] libc: wordexp support
Hello,
This patch adds support for wordexp(3) and wordfree(3) to libc. These
functions conform to IEEE Std 1003.1-2001 (POSIX). The implementation
comes from NetBSD and uses a shell builtin, "wordexp", to perform the
expansion in line with the methods suggested in the specification[1].
Matthew
[1] http://www.opengroup.org/onlinepubs/9699919799/functions/wordexp.html
Index: bin/ksh/c_ksh.c
===
RCS file: /cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.33
diff -N -u -p bin/ksh/c_ksh.c
--- bin/ksh/c_ksh.c 7 Feb 2009 14:03:24 - 1.33
+++ bin/ksh/c_ksh.c 6 Apr 2010 04:36:45 -
@@ -520,6 +520,32 @@ c_whence(char **wp)
return ret;
}
+/*
+ * Do most of the work for wordexp(3). The output is a NULL delimited string
+ * of the format: "\0\0\0\0...\0".
+ */
+int
+c_wordexp(char **wp)
+{
+ unsigned int i, len;
+
+ len = 0;
+
+ if (wp[0] == NULL)
+ return (1);
+
+ for (i = 1; wp[i] != NULL; i++)
+ len += strlen(wp[i]);
+
+ shprintf("%u%c", i - 1, '\0');
+ shprintf("%u%c", len, '\0');
+
+ for (i = 1; wp[i] != NULL; i++)
+ shprintf("%s%c", wp[i], '\0');
+
+ return (0);
+}
+
/* Deal with command -vV - command -p dealt with in comexec() */
int
c_command(char **wp)
@@ -1400,6 +1426,7 @@ const struct builtin kshbuiltins [] = {
{"=typeset", c_typeset},
{"+unalias", c_unalias},
{"whence", c_whence},
+ {"wordexp", c_wordexp},
#ifdef JOBS
{"+bg", c_fgbg},
{"+fg", c_fgbg},
Index: bin/ksh/proto.h
===
RCS file: /cvs/src/bin/ksh/proto.h,v
retrieving revision 1.32
diff -N -u -p bin/ksh/proto.h
--- bin/ksh/proto.h 29 Jan 2009 23:27:26 - 1.32
+++ bin/ksh/proto.h 6 Apr 2010 04:36:45 -
@@ -29,6 +29,7 @@ int c_kill(char **);
void getopts_reset(int);
intc_getopts(char **);
intc_bind(char **);
+intc_wordexp(char **);
/* c_sh.c */
intc_label(char **);
intc_shift(char **);
Index: include/Makefile
===
RCS file: /cvs/src/include/Makefile,v
retrieving revision 1.153
diff -N -u -p include/Makefile
--- include/Makefile3 Feb 2010 20:49:58 - 1.153
+++ include/Makefile6 Apr 2010 04:36:45 -
@@ -23,7 +23,7 @@ FILES=a.out.h ar.h assert.h bitstring.h blf.h bm.h bs
stdbool.h stddef.h stdio.h stdlib.h \
string.h strings.h struct.h sysexits.h tar.h \
time.h ttyent.h tzfile.h unistd.h utime.h utmp.h vis.h \
- wchar.h wctype.h
+ wchar.h wctype.h wordexp.h
FILES+=link.h link_aout.h link_elf.h
Index: include/wordexp.h
===
RCS file: include/wordexp.h
diff -N -u -p include/wordexp.h
--- /dev/null 5 Apr 2010 22:36:46 -
+++ include/wordexp.h 6 Apr 2010 04:36:45 -
@@ -0,0 +1,74 @@
+/* $NetBSD: wordexp.h,v 1.2 2008/04/01 19:23:28 drochner Exp $ */
+
+/*-
+ * Copyright (c) 2002 Tim J. Robbins.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD: /repoman/r/ncvs/src/include/wordexp.h,v 1.4 2003/01/03 12:03:38
tjr Exp $
+ */
+
+#ifndef _WORDEXP_H_
+#define _WORDEXP_H_
+
+#include
+
+typedef struct {
+ size_t we_wordc; /* count of words matched */
+ char**we_wordv; /* pointer to list of words */
+ size_t we_offs;/* slots to reserve in we_wordv */
+
+ /* internal */
+ char*we_strings;/* storage for wordv strings */
+ size_t we_n
