Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

PATCH: Fix use-after-free for print -zf and print -sf

% print -zf hi
 mem.c:1525: BUG: attempt to free already free storage                                        
% XY<81>^A

This is only visible (for me) with --enable-zsh-mem, but the bug is
always there. As the error message says, we do a double free. The reason
is that while we fflush the open_memstream'd fout, which updates buf,
we then after copying the pointer do fclose, which updates the pointer
again and frees the old memory area. Boom.

I don't know what the nicest way to handle this is. We could do what
this patch does, or we could copy the contents of buf after flushing,
but that would cause two extra pointless copies compared to this patch.

We could also set fout = stdout to avoid changing the final if, but I
recall someone complained about the extra fflush of stdout on my other
patch a while back.

 Src/builtin.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Src/builtin.c b/Src/builtin.c
index 19155fd..e0f6a5c 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -4526,7 +4526,8 @@ bin_print(char *name, char **args, Options ops, int func)
     if (OPT_ISSET(ops,'z') || OPT_ISSET(ops,'s')) {
 	putc(0, fout);
-	fflush(fout);
+	fclose(fout);
+	fout = NULL;
 	buf = (char *)zalloc(count + 1);
@@ -4548,7 +4549,7 @@ bin_print(char *name, char **args, Options ops, int func)
     /* Testing EBADF special-cases >&- redirections */
-    if ((fout != stdout) ? (fclose(fout) != 0) :
+    if (fout && (fout != stdout) ? (fclose(fout) != 0) :
 	(fflush(fout) != 0 && errno != EBADF)) {
 	zwarnnam(name, "write error: %e", errno);
 	ret = 1;

Messages sorted by: Reverse Date, Date, Thread, Author