Crash after history/fc -p if history size is set to 0 or non-numerical value

Crash after history/fc -p if history size is set to 0 or non-numerical value

[Axel Beckert]

Hi,

Mika Prokop found the following crash, which I can reproduce with zsh
versions 4.3.10, 4.3.17, 5.0.6 and 5.0.7 (those in Debian):

~ → zsh -f
kiva6% fc -p a b
kiva6% c
[1]    4284 segmentation fault (core dumped)  zsh -f
~ → 

a, b and c can be arbitrary (non-numeric) strings. I suspect that the
cause is that "b" is non-numeric and hence interpreted as zero. Works
with zero instead of "b", too:

~ → zsh -f
kiva6% fc -p a 0
kiva6% foo
[1]    4528 segmentation fault (core dumped)  zsh -f
~ → 

(Yeah, I know, the example with "0" is probably silly, but it still
shouldn't crash. In comparison, setting $HISTSIZE or $SAVEHIST to zero
doesn't crash.)

Backtrace from gdb:

Program received signal SIGSEGV, Segmentation fault.
putoldhistentryontop (keep_going=keep_going@entry=0) at ../../Src/hist.c:1113
1113    ../../Src/hist.c: No such file or directory.
(gdb) bt
#0  putoldhistentryontop (keep_going=keep_going@entry=0) at ../../Src/hist.c:1113
#1  0x0000000000435577 in prepnexthistent () at ../../Src/hist.c:1167
#2  0x0000000000438ff1 in hend (prog=prog@entry=0x7ffff7fe1910) at ../../Src/hist.c:1361
#3  0x000000000043b31d in loop (toplevel=toplevel@entry=1, justonce=justonce@entry=0) at ../../Src/init.c:151
#4  0x000000000043e6f6 in zsh_main (argc=<optimized out>, argv=<optimized out>) at ../../Src/init.c:1625
#5  0x00007ffff7121b45 in __libc_start_main (main=0x40f1a0 <main>, argc=2, argv=0x7fffffffe2e8, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7fffffffe2d8) at libc-start.c:287
#6  0x000000000040f1ce in _start ()

(Backtrace is from 5.0.6, but that shouldn't make much of a
difference, given that I can reproduce this in much older releases,
too.)

		Kind regards, Axel
-- 
/~\  Plain Text Ribbon Campaign                   | Axel Beckert
\ /  Say No to HTML in E-Mail and News            | abe@deuxchevaux.org  (Mail)
 X   See http://www.nonhtmlmail.org/campaign.html | abe@noone.org (Mail+Jabber)
/ \  I love long mails: http://email.is-not-s.ms/ | http://noone.org/abe/ (Web)

PATCH Re: Crash after history/fc -p if history size is set to 0 or non-numerical value

[Bart Schaefer]

On Oct 10, 12:47pm, Axel Beckert wrote:
}
} ~ -> zsh -f
} kiva6% fc -p a b
} kiva6% c
} [1]    4284 segmentation fault (core dumped)  zsh -f

This rejects non-integer values but also handles a zero value.  The hunk
in putoldhistentryontop() is just extra defensive programming, the small
change in prepnexthistent() is the real repair.


diff --git a/Src/builtin.c b/Src/builtin.c
index 4a10c7d..5b711ed 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1363,10 +1363,19 @@ bin_fc(char *nam, char **argv, Options ops, int func)
 	if (*argv) {
 	    hf = *argv++;
 	    if (*argv) {
-		hs = zstrtol(*argv++, NULL, 10);
-		if (*argv)
-		    shs = zstrtol(*argv++, NULL, 10);
-		else
+		char *check;
+		hs = zstrtol(*argv++, &check, 10);
+		if (*check) {
+		    zwarnnam("fc", "HISTSIZE must be an integer");
+		    return 1;
+		}
+		if (*argv) {
+		    shs = zstrtol(*argv++, &check, 10);
+		    if (*check) {
+			zwarnnam("fc", "SAVEHIST must be an integer");
+			return 1;
+		    }
+		} else
 		    shs = hs;
 		if (*argv) {
 		    zwarnnam("fc", "too many arguments");
diff --git a/Src/hist.c b/Src/hist.c
index 4660fd0..0831756 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1110,8 +1110,11 @@ static void
 putoldhistentryontop(short keep_going)
 {
     static Histent next = NULL;
-    Histent he = keep_going? next : hist_ring->down;
-    next = he->down;
+    Histent he = (keep_going || !hist_ring) ? next : hist_ring->down;
+    if (he)
+	next = he->down;
+    else
+	return;
     if (isset(HISTEXPIREDUPSFIRST) && !(he->node.flags & HIST_DUP)) {
 	static zlong max_unique_ct = 0;
 	if (!keep_going)
@@ -1151,7 +1154,7 @@ prepnexthistent(void)
 	freehistnode(&hist_ring->node);
     }
 
-    if (histlinect < histsiz) {
+    if (histlinect < histsiz || !hist_ring) {
 	he = (Histent)zshcalloc(sizeof *he);
 	if (!hist_ring)
 	    hist_ring = he->up = he->down = he;