Created attachment 199366 [details]
lightly redacted core file from sh(1)
bleb on Freenode #freebsd noted that he saw the sh(1) bind(1) when piping it into less(1). I could reproduce it, although it's sporadic. I've also seen it piping into more(1) and redirecting to a file. I see it both both running sh(1) and exec'ing it from my current shell.
$ bind | more
Segmentation fault (core dumped)
I'm attaching the core, with some string subs in place to help obscure the system identity.
CC'ing jilles@ for some triage as behavior is observed in sh(1).
For added flavour:
10:58 < kevans91> It's a multi-release bug at this point
10:58 < mason> Not just 12?
10:58 < kevans91> repro's on this 11.2 box that's restoring stuff
I notice a bug in libedit. The function map_bind() in lib/libedit/map.c assumes that the argv array ends with a NULL string pointer, but the documentation for el_parse() does not say this is required and the implementation of ct_decode_argv() does not make it such. As a result, memory out of bounds of the allocation is accessed.
Since applications calling el_wparse() cannot be assumed to add the NULL sentinel, functions like map_bind() should be adjusted (there may be more places making this incorrect assumption).
By the way, I don't think the approach of converting strings into wchar_t strings (that is, UTF-32, most of the time) should be repeated in new code.
*** Bug 234690 has been marked as a duplicate of this bug. ***
FWIW, an easy way to reproduce seems to be:
$ bind "\e[3~" ed-delete-next-char
I wrote a a patch that fixes the problem mentioned in comment #3.
Just fixed to check the end of argv list by argc counts.
The attached `libedit.patch` is a patch file for this.
And I found another problem that built in 'bind' never redirects to a pipe and/or a file.
The `bind` command always outputs messages to the terminal.
This is because built in `bind` calls el_parse(3) and its outout file descriptor never be changed by pipe and/or redirects.
The attached `sh.patch` fixes this problem.
Could you see my patches and try them?
I checked my patches work on FreeBSD-12.0R.
Created attachment 200904 [details]
Created attachment 200905 [details]
Created attachment 201153 [details]
I found that this problem has already been fixed in NetBSD.
See the following URL.
I updated `libedit.patch` just regenerated for FreeBSD src tree.
A commit references this bug:
Date: Wed Jan 16 21:59:19 UTC 2019
New revision: 343105
libedit: Avoid out of bounds read in 'bind' command
This is CVS revision 1.31 from NetBSD lib/libedit/chartype.c:
Make sure that argv is NULL terminated since functions like tty_stty rely
on it to be so (Gerry Swinslow)
This broke when the wide-character support was enabled in libedit. The
conversion from multibyte to wide-character did not supply the apparently
expected terminating NULL in the new argv array.
Submitted by: Yuichiro NAITO
Obtained from: NetBSD
MFC after: 1 week
As for the sh(1) part, it looks like it should work but I don't really like that it's a workaround for a workaround: sh supplies its standard error as standard output to libedit, so that prompts are written to standard error as required by POSIX. The bind builtin would then temporarily undo this again.
However, something is definitely required to prevent bind's output from staying in a stdio buffer indefinitely. It is also useful to report write errors like other builtins can.
I will think about it for a while.