Segfault because metafy() writes to a const char

Segfault because metafy() writes to a const char

[Theo Buehler]

When trying to load a non-existent module with

% zmodload bla

both with zsh versions 5.0.5 and 5.0.7 on OpenBSD 5.7 (-current), zsh
crashes and dumps core.


Here's a backtrace of the crash

#0  0x0000176db68ab49f in metafy (buf=0x17700e20cfdf "File not found", len=14, heap=1) at utils.c:4116
4116        *e = '\0';
(gdb) bt
#0  0x0000176db68ab49f in metafy (buf=0x17700e20cfdf "File not found", len=14, heap=1) at utils.c:4116
#1  0x0000176db686ad2a in do_load_module (name=0x17702e8d7a38 "bla", silent=0) at module.c:1600
#2  0x0000176db686b9b8 in load_module (name=0x17702e8d7a38 "bla", enablesarr=0x0, silent=0)
    at module.c:2202
#3  0x0000176db686c523 in require_module (module=0x17702e8d7a38 "bla", features=0x0) at module.c:2335
#4  0x0000176db686dfc5 in bin_zmodload_load (nam=0x17702e8d7a28 "zmodload", args=0x17702e8d7a48,
    ops=0x7f7fffff9050) at module.c:2971
#5  0x0000176db686cce2 in bin_zmodload (nam=0x17702e8d7a28 "zmodload", args=0x17702e8d7a48,
    ops=0x7f7fffff9050, func=0) at module.c:2486
#6  0x0000176db6817ff0 in execbuiltin (args=0x17702e8d79e0, bn=0x176db6abb400) at builtin.c:450
#7  0x0000176db6835f19 in execcmd (state=0x7f7fffff9710, input=0, output=0, how=18, last1=2)
    at exec.c:3378
#8  0x0000176db6830e6f in execpline2 (state=0x7f7fffff9710, pcode=131, how=18, input=0, output=0, last1=0)
    at exec.c:1697
#9  0x0000176db68302a8 in execpline (state=0x7f7fffff9710, slcode=4098, how=18, last1=0) at exec.c:1484
#10 0x0000176db682f9e3 in execlist (state=0x7f7fffff9710, dont_change_job=0, exiting=0) at exec.c:1267
#11 0x0000176db682f309 in execode (p=0x17702e8d7918, dont_change_job=0, exiting=0,
    context=0x176db69b35d7 "toplevel") at exec.c:1073
#12 0x0000176db684f1bb in loop (toplevel=1, justonce=0) at init.c:185
#13 0x0000176db6852f1d in zsh_main (argc=1, argv=0x7f7fffff9898) at init.c:1638
#14 0x0000176db68174df in main (argc=1, argv=0x7f7fffff9898) at ./main.c:93
(gdb) %


The following patch fixes this problem for me, however, it effectively
undoes a patch discussed in this thread:


http://www.zsh.org/mla/workers/2013/msg01089.html

and, especially here:

http://www.zsh.org/mla/workers/2013/msg01091.html




--- Src/utils.c.orig    Wed Feb 18 15:32:20 2015
+++ Src/utils.c Wed Feb 18 15:32:44 2015
@@ -4069,7 +4069,7 @@ metafy(char *buf, int len, int heap)
            if (imeta(*e++))
                meta++;

-    if (meta || heap == META_DUP || heap == META_HEAPDUP) {
+    if (meta || heap == META_DUP || heap == META_HEAPDUP || *e != '\0') {
        switch (heap) {
        case META_REALLOC:
            buf = zrealloc(buf, len + meta + 1);
@@ -4112,8 +4112,8 @@ metafy(char *buf, int len, int heap)
                meta--;
            }
        }
+       *e = '\0';
     }
-    *e = '\0';
     return buf;
 }

Re: Segfault because metafy() writes to a const char

[Bart Schaefer]

On Feb 18,  4:30pm, Theo Buehler wrote:
} Subject: Segfault because metafy() writes to a const char
}
} The following patch fixes this problem for me, however, it effectively
} undoes a patch discussed in this thread:
} 
} 
} http://www.zsh.org/mla/workers/2013/msg01089.html
} 
} and, especially here:
} 
} http://www.zsh.org/mla/workers/2013/msg01091.html

If you look at the followup to that message:

http://www.zsh.org/mla/workers/2013/msg01092.html

("we need to change the caller")  The fix should be this:


diff --git a/Src/module.c b/Src/module.c
index 7dd4701..368254c 100644
--- a/Src/module.c
+++ b/Src/module.c
@@ -1601,7 +1601,7 @@ do_load_module(char const *name, int silent)
 #ifdef HAVE_DLERROR
 	char *errstr = dlerror();
 	zwarn("failed to load module `%s': %s", name,
-	      errstr ? metafy(errstr, -1, META_USEHEAP) : "empty module path");
+	      errstr ? metafy(errstr, -1, META_HEAPDUP) : "empty module path");
 #else
 	zwarn("failed to load module: %s", name);
 #endif

(Could probably even use META_STATIC there, the dlerror string is not very
likely to be too long for PATH_MAX.)

It still seems as though we should be able to avoid nul-terminating a
string that's already nul-terminated without changing the call sign of
metafy().

Re: Segfault because metafy() writes to a const char

[Theo Buehler]

On Wed, Feb 18, 2015 at 08:49:41AM -0800, Bart Schaefer wrote:
> On Feb 18,  4:30pm, Theo Buehler wrote:
>
> If you look at the followup to that message:
> 
> http://www.zsh.org/mla/workers/2013/msg01092.html
> 
> ("we need to change the caller")  The fix should be this:
> 
> 
> diff --git a/Src/module.c b/Src/module.c
> index 7dd4701..368254c 100644
> --- a/Src/module.c
> +++ b/Src/module.c
> @@ -1601,7 +1601,7 @@ do_load_module(char const *name, int silent)
>  #ifdef HAVE_DLERROR
>  	char *errstr = dlerror();
>  	zwarn("failed to load module `%s': %s", name,
> -	      errstr ? metafy(errstr, -1, META_USEHEAP) : "empty module path");
> +	      errstr ? metafy(errstr, -1, META_HEAPDUP) : "empty module path");
>  #else
>  	zwarn("failed to load module: %s", name);
>  #endif

Thank you for the immediate response.  This patch indeed seems to be
correct.  It fixes the problem both with zsh 5.0.5 and 5.0.7 on
OpenBSD-current.