NetBSD Problem Report #54399

From www@netbsd.org  Tue Jul 23 09:02:53 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id CE8807A158
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 23 Jul 2019 09:02:53 +0000 (UTC)
Message-Id: <20190723090252.EBDA17A1BF@mollari.NetBSD.org>
Date: Tue, 23 Jul 2019 09:02:52 +0000 (UTC)
From: soeren+netbsd@soeren-tempel.net
Reply-To: soeren+netbsd@soeren-tempel.net
To: gnats-bugs@NetBSD.org
Subject: [PATCH] Uninitialized memory access in libedit
X-Send-Pr-Version: www-1.0

>Number:         54399
>Category:       lib
>Synopsis:       [PATCH] Uninitialized memory access in libedit
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    christos
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 23 09:05:01 +0000 2019
>Last-Modified:  Mon Aug 05 19:05:01 +0000 2019
>Originator:     Sören Tempel
>Release:        libedit from 2019-03-24
>Organization:
>Environment:
Unfortunately, I use the portable version of netbsd libedit that Jess Thrysĝe distributes on Linux. However, I verified that the problem wasn't fix in NetBSD CVS yet.
>Description:
I believe I found an uninitialized memory access in libedit's hist_get
function. The uninitialized memory access is due to the fact that the
history initialization function (hist_init) doesn't initialize the
memory for el->el_history.buf. However hist_get makes the assumption
that el->el_history.buf is always null-terminated.

If hist_get is called directly after hist_init it accesses
el->el_history.buf, even though it is not initialized, when invoking
wcsncpy(el->el_history.buf, el->el_line.buffer, EL_BUFSIZ). This is,
for instance, the case when hist_get is called through ed_next_history
using ^N directly after starting a history-enabled libedit program.
>How-To-Repeat:
Use a history-enabled program linked against libedit, start it in valgrind and immediately press Ctrl+N. Example with sftp from OpenSSH:

$ valgrind sftp <host>
sftp>
==32040== Conditional jump or move depends on uninitialised value(s)
==32040==    at 0x404ED2B: wcsncpy (wcsncpy.c:6)
==32040==    by 0x4F35376: wcsncpy (wchar.h:169)
==32040==    by 0x4F35376: hist_get (hist.c:108)
==32040==    by 0x4F32E69: ed_next_history (common.c:610)
==32040==    by 0x4F37DB3: el_wgets (read.c:538)
==32040==    by 0x4F34287: el_gets (eln.c:75)
==32040==    by 0x1118F5: ??? (in /usr/bin/sftp)
==32040==    by 0x10D070: ??? (in /usr/bin/sftp)
==32040==    by 0x401C230: libc_start_main_stage2 (__libc_start_main.c:94)
==32040==    by 0x10D115: ??? (in /usr/bin/sftp)
==32040==    by 0x1: ???
==32040==    by 0x1FFF000B9E: ???
==32040==    by 0x1FFF000BA3: ???
>Fix:
The following patch uses calloc(3) instead of malloc(3) in hist_init to fix the problem. Using memset after invoking malloc would also be possible. Not sure what you prefer.

diff -upr libedit-20190324-3.1.orig/src/hist.c libedit-20190324-3.1/src/hist.c
--- libedit-20190324-3.1.orig/src/hist.c        2019-07-20 21:19:08.374826681 +0200
+++ libedit-20190324-3.1/src/hist.c     2019-07-20 21:28:43.394825734 +0200
@@ -59,7 +59,7 @@ hist_init(EditLine *el)

        el->el_history.fun = NULL;
        el->el_history.ref = NULL;
-       el->el_history.buf = el_malloc(EL_BUFSIZ * sizeof(*el->el_history.buf));
+       el->el_history.buf = calloc(EL_BUFSIZ, sizeof(*el->el_history.buf));
        el->el_history.sz  = EL_BUFSIZ;
        if (el->el_history.buf == NULL)
                return -1;

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54399 CVS commit: src/lib/libedit
Date: Tue, 23 Jul 2019 06:18:53 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Tue Jul 23 10:18:52 UTC 2019

 Modified Files:
 	src/lib/libedit: chared.c chartype.c el.c el.h filecomplete.c hist.c
 	    keymacro.c literal.c map.c parse.c read.c readline.c search.c
 	    terminal.c vi.c

 Log Message:
 PR/54399: Sören Tempel: Uninitialized memory access in libedit history.
 Initialize the buffer using calloc. While here change all malloc(a * sizeof(b))
 to calloc(a, sizeof(b)). XXX: should fix realloc similarly.


 To generate a diff of this commit:
 cvs rdiff -u -r1.58 -r1.59 src/lib/libedit/chared.c
 cvs rdiff -u -r1.34 -r1.35 src/lib/libedit/chartype.c
 cvs rdiff -u -r1.98 -r1.99 src/lib/libedit/el.c
 cvs rdiff -u -r1.44 -r1.45 src/lib/libedit/el.h
 cvs rdiff -u -r1.55 -r1.56 src/lib/libedit/filecomplete.c
 cvs rdiff -u -r1.32 -r1.33 src/lib/libedit/hist.c
 cvs rdiff -u -r1.23 -r1.24 src/lib/libedit/keymacro.c
 cvs rdiff -u -r1.3 -r1.4 src/lib/libedit/literal.c
 cvs rdiff -u -r1.51 -r1.52 src/lib/libedit/map.c
 cvs rdiff -u -r1.41 -r1.42 src/lib/libedit/parse.c
 cvs rdiff -u -r1.105 -r1.106 src/lib/libedit/read.c
 cvs rdiff -u -r1.155 -r1.156 src/lib/libedit/readline.c
 cvs rdiff -u -r1.48 -r1.49 src/lib/libedit/search.c
 cvs rdiff -u -r1.38 -r1.39 src/lib/libedit/terminal.c
 cvs rdiff -u -r1.62 -r1.63 src/lib/libedit/vi.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: lib-bug-people->christos
Responsible-Changed-By: maya@NetBSD.org
Responsible-Changed-When: Mon, 05 Aug 2019 19:00:01 +0000
Responsible-Changed-Why:


From: maya@NetBSD.org
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/54399 ([PATCH] Uninitialized memory access in libedit)
Date: Mon, 5 Aug 2019 19:01:08 +0000

 Error in editing, sorry!
 Would you like to keep the bug open for
 "XXX: should fix realloc similarly"?

>Unformatted:

Home	PR Database Search