From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12014 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: posix_spawnp stack overflow/corruption by child when PATH is large? Date: Thu, 19 Oct 2017 17:10:13 -0400 Message-ID: <20171019211013.GR1627@brightrain.aerifal.cx> References: <20170915141742.GW1627@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1508447432 16086 195.159.176.226 (19 Oct 2017 21:10:32 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 19 Oct 2017 21:10:32 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12027-gllmg-musl=m.gmane.org@lists.openwall.com Thu Oct 19 23:10:28 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 1e5I56-0002fV-Ky for gllmg-musl@m.gmane.org; Thu, 19 Oct 2017 23:10:20 +0200 Original-Received: (qmail 9451 invoked by uid 550); 19 Oct 2017 21:10:25 -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 9431 invoked from network); 19 Oct 2017 21:10:25 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12014 Archived-At: On Thu, Oct 19, 2017 at 04:05:19PM -0500, Will Dietz wrote: > (soft ping) Oops, I probably should have gotten this into the release. At least it makes a good motivation to pick up the release pace and make another release soon. Rich > On Mon, Sep 18, 2017 at 2:31 PM, Will Dietz wrote: > > 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