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

[Takashi Iwai]

On Wed, 12 Jul 2017 14:54:08 +0200,
Natanael Copa wrote:
> 
> 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.

Yeah, I *guess* it suffices in most cases, but who knows what kind of
weird setup users have :)


thanks,

Takashi