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@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.
Signed-off-by: Natanael Copa ncopa@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