lib/54400: [PATCH] out-of-bounds read in libedit

[soeren+netbsd%soeren-tempel.net@localhost]

>Number:         54400
>Category:       lib
>Synopsis:       [PATCH] out-of-bounds read in libedit
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 23 09:35: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 fixed in NetBSD CVS yet.
>Description:
I believe I found an buffer overread in libedit. The bug is caused by a broken bounds check in `c_delbefore` from `chared.c`.
>How-To-Repeat:
Use a program linked against libedit with `bind -e` in `~/.editrc`. Start it in valgrind, enter a large number of characters, press Ctrl+w.

Example with sftp from OpenSSH:

$ valgrind sftp <host>
sftp> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
==32468== Invalid read of size 4
==32468==    at 0x4F31E96: c_delbefore (chared.c:179)
==32468==    by 0x4F32A00: ed_delete_prev_word (common.c:133)
==32468==    by 0x4F37DB3: el_wgets (read.c:538)
==32468==    by 0x4F34287: el_gets (eln.c:75)
==32468==    by 0x1118F5: ??? (in /usr/bin/sftp)
==32468==    by 0x10D070: ??? (in /usr/bin/sftp)
==32468==    by 0x401C230: libc_start_main_stage2 (__libc_start_main.c:94)
==32468==    by 0x10D115: ??? (in /usr/bin/sftp)
==32468==    by 0x1: ???
==32468==    by 0x1FFF000B9E: ???
==32468==    by 0x1FFF000BA3: ???
==32468==  Address 0x49520b0 is 0 bytes after a block of size 8,192 alloc'd
==32468==    at 0x48A08C2: realloc (vg_replace_malloc.c:836)
==32468==    by 0x4F323D5: ch_enlargebufs (chared.c:505)
==32468==    by 0x4F32912: ed_insert (common.c:86)
==32468==    by 0x4F37DB3: el_wgets (read.c:538)
==32468==    by 0x4F34287: el_gets (eln.c:75)
==32468==    by 0x1118F5: ??? (in /usr/bin/sftp)
==32468==    by 0x10D070: ??? (in /usr/bin/sftp)
==32468==    by 0x401C230: libc_start_main_stage2 (__libc_start_main.c:94)
==32468==    by 0x10D115: ??? (in /usr/bin/sftp)
==32468==    by 0x1: ???
==32468==    by 0x1FFF000B9E: ???
==32468==    by 0x1FFF000BA3: ???
==32468== 
==32468== Invalid read of size 4
==32468==    at 0x4F31E94: c_delbefore (chared.c:176)
==32468==    by 0x4F32A00: ed_delete_prev_word (common.c:133)
==32468==    by 0x4F37DB3: el_wgets (read.c:538)
==32468==    by 0x4F34287: el_gets (eln.c:75)
==32468==    by 0x1118F5: ??? (in /usr/bin/sftp)
==32468==    by 0x10D070: ??? (in /usr/bin/sftp)
==32468==    by 0x401C230: libc_start_main_stage2 (__libc_start_main.c:94)
==32468==    by 0x10D115: ??? (in /usr/bin/sftp)
==32468==    by 0x1: ???
==32468==    by 0x1FFF000B9E: ???
==32468==    by 0x1FFF000BA3: ???
==32468==  Address 0x49520b4 is 4 bytes after a block of size 8,192 alloc'd
==32468==    at 0x48A08C2: realloc (vg_replace_malloc.c:836)
>Fix:
The following patch fixes the bounds check in c_delbefore. A similar idiom is also used in `c_insert` and `c_delafter` these might need to be fixed as well. You might also want to consider replacing the entire for-loop with memmove(3).

diff -upr libedit-20190324-3.1.orig/src/chared.c libedit-20190324-3.1/src/chared.c
--- libedit-20190324-3.1.orig/src/chared.c	2019-07-23 11:23:06.774645695 +0200
+++ libedit-20190324-3.1/src/chared.c	2019-07-23 11:23:27.841331723 +0200
@@ -174,7 +174,7 @@ c_delbefore(EditLine *el, int num)
 		wchar_t *cp;
 
 		for (cp = el->el_line.cursor - num;
-		    cp <= el->el_line.lastchar;
+		    &cp[num] <= el->el_line.lastchar;
 		    cp++)
 			*cp = cp[num];