Re: [patch] libc: wordexp support

2010-04-06 Thread Ted Unangst
On Tue, Apr 6, 2010 at 1:53 AM, Matthew Haub
matthew.h...@alumni.adelaide.edu.au 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 newline 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

2010-04-06 Thread Theo de Raadt
I think we should stand up to crap and not ever impliment it.

 On Tue, Apr 6, 2010 at 1:53 AM, Matthew Haub
 matthew.h...@alumni.adelaide.edu.au 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 newline 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

2010-04-06 Thread Theo de Raadt
 On Tue, Apr 6, 2010 at 1:53 AM, Matthew Haub
 matthew.h...@alumni.adelaide.edu.au 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 newline 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().