[alsa-devel] [PATCH - alsa-lib 1/1] snd_user_file: avoid use wordexp

[Natanael Copa]

Hi,

Thanks for your quick response!

On Wed, 12 Jul 2017 12:02:28 +0200
Takashi Iwai <tiwai at suse.de> wrote:

> On Wed, 12 Jul 2017 09:53:08 +0200,
> Natanael Copa wrote:
> > 
> > As suggested in POSIX[1], wordexp might execute the shell. If the libc
> > implementation does so, it will break the firefox sandbox which does
> > not allow exec. This happened on Alpine Linux with musl libc[2].
> > 
> > Since we cannot guarantee that the system wordexp implementation does
> > not execute shell, we cannot really use it, and need to implement the
> > ~/ expansion ourselves.
> > 
> > Generally, wordexp is a large attack vector and it is better to avoid it
> > since only tilde expansion is needed.
> > 
> > [1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/wordexp.html#tag_16_684_08
> > [2]: http://bugs.alpinelinux.org/issues/7454#note-2
> > 
> > Signed-off-by: Natanael Copa <ncopa at alpinelinux.org>  
> 
> Well, this may bring a functional regression, so I don't want to
> introduce this unconditionally.  Instead, we may introduce configure
> option to explicitly control the usage of wordexp(), and add your
> extension to the fallback code, for example.  If the security issue
> really matters, we can leave it disabled as default, too.

I just tested and can confirm that it does break "$HOME/.asoundrc"
which previously worked. I don't know if it is needed/desired but I
would guess that environment variables should be expanded with "@func
getenv" and that expansion of environment vars without @func getenv was
unintentional.

The command expansion is explicitly disabled with WRDE_NOCMD so that
means that things like "$(pwd)/.asoundrc" never worked.

Wildcard expansion did work sort of, but would only pick the first
match and not return all matches, so it was not really useful or
reliable.

So my conclusion was that the only needed expansion was ~/. This could
been implemented with glob() and GLOB_TILDE, but it is not supported in
POSIX (and ironically, POSIX says that you should use wordexp for tilde
expansion :)

I understand your worry for a functional regression, even if that
functionality is weird and unintended so I will prepare a patch with a
configure option for it.

Another option would be to drop support for ~/ expansion and use
@func getenv to expand $HOME, but not sure how that is done. I suppose
expanding ~/ directly like my patches does is fine.


Thanks!

-nc