From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11949 Path: news.gmane.org!.POSTED!not-for-mail From: Will Dietz Newsgroups: gmane.linux.lib.musl.general Subject: Re: posix_spawnp stack overflow/corruption by child when PATH is large? Date: Mon, 18 Sep 2017 14:31:25 -0500 Message-ID: References: <20170915141742.GW1627@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="001a113d3ce4dad3c805597bc90a" X-Trace: blaine.gmane.org 1505763099 14543 195.159.176.226 (18 Sep 2017 19:31:39 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 18 Sep 2017 19:31:39 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-11962-gllmg-musl=m.gmane.org@lists.openwall.com Mon Sep 18 21:31:35 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1du1lX-0003Vw-De for gllmg-musl@m.gmane.org; Mon, 18 Sep 2017 21:31:35 +0200 Original-Received: (qmail 13871 invoked by uid 550); 18 Sep 2017 19:31:39 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 13849 invoked from network); 18 Sep 2017 19:31:38 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wdtz.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=nFl9Vr8SKEyvWoFiEiLhcH6aYVOlXgj3jIAyYe1J/v8=; b=T6GikqvJv5l+akK/RGtxNB0q94wq+CH/bL1QlN0YEA1duzmZtwSR3jYPzNfv7wMH8Z 2VcQUngQOVPR5bhaAzdRuil/pLcPIRwYOZQGy9oQ7zs+54iwamCKDPvgdplbr/7gDXfV qNuHtS0BntBCbOUA97rP0layaNAR3wLFLl4x0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=nFl9Vr8SKEyvWoFiEiLhcH6aYVOlXgj3jIAyYe1J/v8=; b=f4qJ1/VjZnZSuKe1hYSCnDqhElZe08ei7bESwf2/f9+qpxdiLFc3RTD5HNJioZsbWF zfZGccjq1blIpsIVFbA8ilwHGBDiXzIkKOsZc4JAmXliWtimGul695aCsL423MkcV7rr zXsDKcKO3MuMU6VZd60jfpzt1jkGm3oVgafRtwm0vwgfGze56UgWz17uxVu8n9sOTXtJ FA+4skLHpZDdVZPS+PmkjVgJ/IU/H7Ke0lXIZhu4ZFkW0jI0TDrFX4MIoRjKd/czV/8r bE/8csKD5qmUYW+8RqFhas/NF1STBkqdvXY+wJTBf7PwGv1Sdl+zZhxS+9kuDbhHtZCL GJrg== X-Gm-Message-State: AHPjjUgwI97yjFlN+b3GVXXlWLPmHqoFvMtIVKdNNuG32+uySDsJyL3d pMyCtvbxO9GbguCHDd3N9pLvXXHqfj1swFDlP6ee/tU= X-Google-Smtp-Source: AOwi7QDyzFDND3SJLrPHB4ZZlyjGRGc6C1wDHFh5hAp6fX8A61Qf3H84CBaMgk2+42SqAWrMLByLEhH/2lS+RqN8TmU= X-Received: by 10.202.206.70 with SMTP id e67mr35407906oig.118.1505763085929; Mon, 18 Sep 2017 12:31:25 -0700 (PDT) X-Originating-IP: [2602:306:304a:61c8:679c:aaad:1e62:8d52] In-Reply-To: <20170915141742.GW1627@brightrain.aerifal.cx> Xref: news.gmane.org gmane.linux.lib.musl.general:11949 Archived-At: --001a113d3ce4dad3c805597bc90a Content-Type: text/plain; charset="UTF-8" Thanks for taking a look and for the confirmation! I agree that 1024+PATH_MAX would be a reasonable value here, good call. I had similar thought about making the extra stack usage conditional, but would rather keep it simple and clear-- as weighed against my possibly wrong "expectation" that the difference won't be significant for folks. I don't feel strongly about it and of course defer to your judgement :). Patch making the discussed change is attached. ~Will On Fri, Sep 15, 2017 at 9:17 AM, Rich Felker wrote: > On Thu, Sep 14, 2017 at 03:39:35PM -0500, Will Dietz wrote: >> Hi, >> >> I believe there is a bug in posix_spawn/execvpe, please take a look and confirm >> or kindly let me know if I'm mistaken and accept my apologies :). >> >> It looks like __posix_spawnx calls clone() with a 1024-byte stack buffer >> (allocated from its own stack), which is insufficient to handle stack >> allocations performed >> in execvpe which are something around a few bytes more than NAME_MAX+PATH_MAX. >> >> This path is taken when using posix_spawnp, and the problem exists on >> 1.1.16 and latest git. >> >> For what it's worth I tracked this down from a crash in 'bison' when >> invoking m4, >> but I've had success reproducing it with the following demo program >> and driver script: >> >> ------------------------------------------- >> #include >> #include >> #include >> #include >> #include >> >> extern char **environ; >> >> int main() { >> >> pid_t p; >> char *argv[] = {"sh", "-c", "echo Hello", NULL}; >> int s, status; >> s = posix_spawnp(&p, "sh", NULL, NULL, argv, environ); >> if (s) { >> perror("posix_spawn"); >> exit(1); >> } >> >> s = waitpid(p, &status, 0); >> >> printf("pid: %d, s: %d, status: %d\n", p, s, status); >> >> return 0; >> } >> -------------- >> >> And little shell script to create a suitably large PATH (mostly to >> demonstrate what I mean, not for unmodified use): >> --------------- >> #!/bin/sh >> >> SLASH_100_As="/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" >> SUFFIX="/123456789012345678901234567" #1234567890" #1234567890" >> >> VAR="/bin:$SUFFIX" >> for x in `seq 10`; do >> VAR="${SLASH_100_As}:$VAR" >> done >> >> echo $VAR >> echo $VAR|wc -c >> >> # Works fine with normal PATH >> ~/cur/musl-spawn/test >> ~/cur/musl-spawn/test >> >> # Crashes when PATH is ~1050 characters >> PATH=$VAR \ >> ~/cur/musl-spawn/test >> -------------- >> >> Where "~/cur/musl-spawn/test" is the test program compiled against musl. >> >> I cannot speak regarding any security implications, but since this may >> grant some measure of stack-scribbling-powers it seems to warrant >> being given brief attention in this context. >> >> An easy fix is to bump the size of the 'char stack[1024]' in >> src/process/posix_spawn.c to a suitable value-- 8096 is overkill but >> does the trick, for example. >> >> Please let me know if I'm missing something or if details are not clear. > > It's very clear, and this seems pretty serious. 1024+PATH_MAX would > probably be a safe limit. If we care about minimal stack usage when > plain posix_spawn (not spawnp) is called, it could be something like > "exec==execve ? 1024 : 1024+PATH_MAX", perhaps. > > Rich --001a113d3ce4dad3c805597bc90a Content-Type: text/x-patch; charset="US-ASCII"; name="0001-posix_spawn-use-larger-stack-to-cover-worst-case-in-.patch" Content-Disposition: attachment; filename="0001-posix_spawn-use-larger-stack-to-cover-worst-case-in-.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_j7qk5hne0 RnJvbSA5YjVjYTU0MWIyYzk3ODUwYjAwYTA1MWFkMjFlZmM0Njc5MmI5MWIyIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBXaWxsIERpZXR6IDx3QHdkdHoub3JnPgpEYXRlOiBUaHUsIDE0 IFNlcCAyMDE3IDE2OjMyOjU5IC0wNTAwClN1YmplY3Q6IFtQQVRDSF0gcG9zaXhfc3Bhd246IHVz ZSBsYXJnZXIgc3RhY2sgdG8gY292ZXIgd29yc3QtY2FzZSBpbiBleGVjdnBlCgpleGVjdnBlIHN0 YWNrLWFsbG9jYXRlcyBhIGJ1ZmZlciB1c2VkIHRvIGhvbGQgdGhlIGZ1bGwgcGF0aAooY29tYmlu YXRpb24gb2YgYSBQQVRIIGVudHJ5IGFuZCB0aGUgcHJvZ3JhbSBuYW1lKQp3aGlsZSBzZWFyY2hp bmcgdGhyb3VnaCAkUEFUSCwgc28gYXQgbGVhc3QKTkFNRV9NQVgrUEFUSF9NQVggaXMgbmVlZGVk LgoKVGhlIHN0YWNrIHNpemUgY2FuIGJlIG1hZGUgY29uZGl0aW9uYWxseSBzbWFsbGVyCih0aGUg Y3VycmVudCAxMDI0IGFwcGVhcnMgYXBwcm9wcmlhdGUpCnNob3VsZCB0aGlzIGxhcmdlciBzaXpl IGJlIGJ1cmRlbnNvbWUgaW4gdGhvc2Ugc2l0dWF0aW9ucy4KLS0tCiBzcmMvcHJvY2Vzcy9wb3Np eF9zcGF3bi5jIHwgMiArLQogMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0 aW9uKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL3Byb2Nlc3MvcG9zaXhfc3Bhd24uYyBiL3NyYy9wcm9j ZXNzL3Bvc2l4X3NwYXduLmMKaW5kZXggZWE1ZDI5OTguLjA4NDljNzFmIDEwMDY0NAotLS0gYS9z cmMvcHJvY2Vzcy9wb3NpeF9zcGF3bi5jCisrKyBiL3NyYy9wcm9jZXNzL3Bvc2l4X3NwYXduLmMK QEAgLTE1Miw3ICsxNTIsNyBAQCBpbnQgX19wb3NpeF9zcGF3bngocGlkX3QgKnJlc3RyaWN0IHJl cywgY29uc3QgY2hhciAqcmVzdHJpY3QgcGF0aCwKIAljaGFyICpjb25zdCBhcmd2W3Jlc3RyaWN0 XSwgY2hhciAqY29uc3QgZW52cFtyZXN0cmljdF0pCiB7CiAJcGlkX3QgcGlkOwotCWNoYXIgc3Rh Y2tbMTAyNF07CisJY2hhciBzdGFja1sxMDI0K1BBVEhfTUFYXTsKIAlpbnQgZWM9MCwgY3M7CiAJ c3RydWN0IGFyZ3MgYXJnczsKIAotLSAKMi4xNC4xCgo= --001a113d3ce4dad3c805597bc90a--