From patchwork Sun Apr 8 16:13:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Hills X-Patchwork-Id: 10328371 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 86C7A6037F for ; Sun, 8 Apr 2018 16:14:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7552F28A6C for ; Sun, 8 Apr 2018 16:14:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 69E3328A6F; Sun, 8 Apr 2018 16:14:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 51CB428A6C for ; Sun, 8 Apr 2018 16:13:57 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 6E3012673C7; Sun, 8 Apr 2018 18:13:55 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id A85D42673C8; Sun, 8 Apr 2018 18:13:52 +0200 (CEST) Received: from jazz.pogo.org.uk (jazz.pogo.org.uk [213.138.114.167]) by alsa0.perex.cz (Postfix) with ESMTP id 14B25267353 for ; Sun, 8 Apr 2018 18:13:47 +0200 (CEST) Received: from cpc93782-hari17-2-0-cust701.20-2.cable.virginm.net ([82.34.66.190] helo=stax.localdomain) by jazz.pogo.org.uk with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90 (FreeBSD)) (envelope-from ) id 1f5Cwp-000Bus-Bp; Sun, 08 Apr 2018 17:13:43 +0100 Received: from mark (helo=localhost) by stax.localdomain with local-esmtp (Exim 4.84) (envelope-from ) id 1f5Cwp-0000YH-1r; Sun, 08 Apr 2018 17:13:43 +0100 Date: Sun, 8 Apr 2018 17:13:43 +0100 (BST) From: Mark Hills To: alsa-devel@alsa-project.org Message-ID: <1804081611300.19491@stax.localdomain> MIME-Version: 1.0 Cc: Takashi Iwai , Natanael Copa Subject: [alsa-devel] Avoiding wordexp prevents environment variables being used X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP I just came up against the patch below; it prevents useful snippets of alsa-conf like this: @hooks [ { func load files [ "~/.asoundrc-$HOSTNAME" ] errors false } ] as the evalutation of all but "~" has been removed. Seems like removal of a perfectly good feature in the name of security; because wordexp() 1) is not used (and should not be used) on data originating from an untrusted source 2) is already used with WRDE_NOCMD, which the same POSIX spec documents as: "The WRDE_NOCMD flag is provided for applications that, for security or other reasons, want to prevent a user from executing shell commands." 3) on glibc can be seen (with strace) not to execute other commands If one is to treat the POSIX doc as gospel (as cited by the patch) the cause of firefox (circa July 2017) not working would actually be that musl does not honour WRDE_NOCMD to the letter. I agree the spec of wordexp() could be more useful, though. Also, hypothesising the attacks of an already-compromised application would get into a sticky conversation about the thread safety of getenv("HOME") (and associated buffer wrangling) vs. a library function being used for its intended purpose. In practice, Firefox may have moved on here (no ALSA support anymore) so should quirks of its sandbox be driving this? diff --git a/configure.ac b/configure.ac index 26e5d125..fbcfa829 100644 --- a/configure.ac +++ b/configure.ac @@ -303,8 +303,25 @@ fi AC_SUBST(ALSA_DEPLIBS) +dnl Check for use of wordexp... +AC_MSG_CHECKING(for use of wordexp) +AC_ARG_WITH(wordexp, + AS_HELP_STRING([--with-wordexp], + [Use wordexp when expanding configs (default = no)]), + [case "$withval" in + y|yes) wordexp=yes ;; + *) wordexp=no ;; + esac],) +if test "$wordexp" = "yes" ; then + AC_DEFINE(HAVE_WORDEXP, "1", [Enable use of wordexp]) + AC_MSG_RESULT(yes) + AC_CHECK_HEADER([wordexp.h],[], [AC_MSG_ERROR([Couldn't find wordexp.h])]) +else + AC_MSG_RESULT(no) +fi + dnl Check for headers -AC_CHECK_HEADERS([wordexp.h endian.h sys/endian.h sys/shm.h]) +AC_CHECK_HEADERS([endian.h sys/endian.h sys/shm.h]) dnl Check for resmgr support... AC_MSG_CHECKING(for resmgr support) diff --git a/src/userfile.c b/src/userfile.c index 72779da4..f2145470 100644 --- a/src/userfile.c +++ b/src/userfile.c @@ -21,6 +21,7 @@ #include #include #include +#include /** * \brief Get the full file name @@ -32,14 +33,13 @@ * stores the first matchine one. The returned string is strdup'ed. */ -#ifdef HAVE_WORDEXP_H +#ifdef HAVE_WORDEXP #include -#include int snd_user_file(const char *file, char **result) { wordexp_t we; int err; - + assert(file && result); err = wordexp(file, &we, WRDE_NOCMD); switch (err) { @@ -61,13 +61,62 @@ int snd_user_file(const char *file, char **result) return 0; } -#else /* !HAVE_WORDEXP_H */ -/* just copy the string - would be nicer to expand by ourselves, though... */ +#else /* !HAVE_WORDEX */ + +#include +#include +#include +#include +#include + int snd_user_file(const char *file, char **result) { - *result = strdup(file); - if (! *result) + int err; + size_t len; + char *buf = NULL; + + assert(file && result); + *result = NULL; + + /* expand ~/ if needed */ + if (file[0] == '~' && file[1] == '/') { + const char *home = getenv("HOME"); + if (home == NULL) { + struct passwd pwent, *p = NULL; + uid_t id = getuid(); + size_t bufsize = 1024; + + buf = malloc(bufsize); + if (buf == NULL) + goto out; + + while ((err = getpwuid_r(id, &pwent, buf, bufsize, &p)) == ERANGE) { + char *newbuf; + bufsize += 1024; + if (bufsize < 1024) + break; + newbuf = realloc(buf, bufsize); + if (newbuf == NULL) + goto out; + buf = newbuf; + } + home = err ? "" : pwent.pw_dir; + } + len = strlen(home) + strlen(&file[2]) + 2; + *result = malloc(len); + if (*result) + snprintf(*result, len, "%s/%s", home, &file[2]); + } else { + *result = strdup(file); + } + +out: + if (buf) + free(buf); + + if (*result == NULL) return -ENOMEM; return 0; } -#endif /* HAVE_WORDEXP_H */ + +#endif /* HAVE_WORDEXP */