From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-30111-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 28799 invoked by alias); 16 Jan 2012 14:03:04 -0000
Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
List-Id: Zsh Workers List <zsh-workers.zsh.org>
List-Post: <mailto:zsh-workers@zsh.org>
List-Help: <mailto:zsh-workers-help@zsh.org>
X-Seq: 30111
Received: (qmail 21378 invoked from network); 16 Jan 2012 14:03:01 -0000
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au
X-Spam-Level: 
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,
	SPF_HELO_PASS autolearn=ham version=3.3.2
Received-SPF: none (ns1.primenet.com.au: domain at bewatermyfriend.org does not designate permitted sender hosts)
From: Frank Terbeck <ft@bewatermyfriend.org>
To: zsh-workers@zsh.org
Cc: Phil Pennock <zsh-workers+phil.pennock@spodhuis.org>
Subject: PATCH: Fix segfaults with exec options
User-Agent: Gnus/5.110018 (No Gnus v0.18) Emacs/24.0.91 (gnu/linux)
Date: Mon, 16 Jan 2012 14:55:11 +0100
Message-ID: <87ty3v92gw.fsf@ft.bewatermyfriend.org>
MIME-Version: 1.0
Content-Type: text/plain
X-Df-Sender: [pbs]MDExNTM1

Hey list,

Someone on IRC noticed that zsh flakes out, if you're using the `exec'
builtin with somewhat bogus options. Like this:

% exec -a foo
% exec -l
% exec -c

Turns out that the issue is located in `execcmd()' in `exec.c'. The code
does "nextnode(firstnode(args))" a couple of times and it turns out,
that if used like above (which doesn't make sense, but crashing is
hardly the proper way to react) "firstnode(args)" can be NULL and when
that is accessed by the `nextnode' macro, the shell crashes.

There's a similar problem, when "firstnode(args)" is passed to
`uremnode()', which accesses the memory at that position without testing
for NULL, too.

I've prepared a rather hackish patch that seems to fix the issue.

I've CC:ed Phil, who appears to be the original author of the code
because I don't know the code very well and `execcmd()', being the 1000+
lines function it is, intimidates me a little...

Maybe he (or someone else) has an idea about how to fix this at a more
fundamental level. To me, my fix just plugs some holes with duct tape.

Regards, Frank


diff --git a/Src/exec.c b/Src/exec.c
index 9b3c503..e1bf881 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2487,6 +2487,11 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		 * with the zsh style.
 		 */
 		while (next && *next == '-' && strlen(next) >= 2) {
+		    if (!firstnode(args)) {
+			zerr("exec requires command to execute");
+			errflag = lastval = 1;
+			return;
+		    }
 		    uremnode(args, firstnode(args));
 		    if (!strcmp(next, "--"))
 			break;
@@ -2499,6 +2504,11 @@ execcmd(Estate state, int input, int output, int how, int last1)
 				/* position on last non-NULL character */
 				cmdopt += strlen(cmdopt+1);
 			    } else {
+				if (!firstnode(args)) {
+				    zerr("exec requires command to execute");
+				    errflag = lastval = 1;
+				    return;
+				}
 				if (!nextnode(firstnode(args))) {
 				    zerr("exec flag -a requires a parameter");
 				    errflag = lastval = 1;
@@ -2521,7 +2531,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 			    return;
 			}
 		    }
-		    next = (char *) getdata(nextnode(firstnode(args)));
+		    if (firstnode(args) && nextnode(firstnode(args)))
+			next = (char *) getdata(nextnode(firstnode(args)));
 		}
 		if (exec_argv0) {
 		    char *str, *s;