Date: Mon, 25 Apr 2005 23:32:16 GMT From: "Wojciech A. Koszek" <dunstan@freebsd.czest.pl> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/80346: [PATCH] Misuse of el_init() can lead multiple programs to SIGSEGV Message-ID: <200504252332.j3PNWGLG003380@freebsd.czest.pl> Resent-Message-ID: <200504252330.j3PNUGg4052630@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 80346
>Category: bin
>Synopsis: [PATCH] Misuse of el_init() can lead multiple programs to SIGSEGV
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Mon Apr 25 23:30:15 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: Wojciech A. Koszek
>Release: FreeBSD 5.3-RELEASE-p5 i386
>Organization:
>Environment:
System: FreeBSD dunstan.freebsd.czest.pl 5.3-RELEASE-p5 FreeBSD 5.3-RELEASE-p5 #4: Sun Mar 6 10:50:43 CET 2005 dunstan@ho:/usr/obj/usr/src/sys/HOME7 i386
>Description:
There is a problem in multiple programs which make use of editline library
which may lead software to crash.
Code placed in sys/kern/tty.c makes it possible to set TTY size to arbitrary
values. Number of rows and columns is kept in unsigned short variables.
Setting the biggest values for those variables is possible with stty(1):
$ stty rows -1
$ stty columns -1
After running applications with changed terminal size, further allocations
are done on given (very big) size. Allocation may not succeed and
applications which don't check return value of el_init() continue executing,
and thus, get SIGSEGV. This problem may have security implications --
programs linked with editline run with raised privileges (lpc). Known
problems:
src/usr.sbin/lpr/lpc/lpc.c
[..]
if (!el) {
el = el_init("lpc", stdin, stdout, stderr);
hist = history_init();
history(hist, &he, H_EVENT, 100);
el_set(el, EL_HIST, history, hist);
[..]
src/usr.bin/tftp/main.c:
[..]
vrbose = isatty(0);
if (vrbose) {
el = el_init("tftp", stdin, stdout, stderr);
hist = history_init();
}
[..]
src/usr.sbin/cdcontrol/cdcontrol.c
[..]
if (!el) {
el = el_init("cdcontrol", stdin, stdout, stderr);
hist = history_init();
history(hist, &he, H_EVENT, 100);
[..]
src/usr.sbin/pppctl/pppctl.c
[..]
history(td.hist, &hev, H_SETSIZE, size);
td.edit = el_init("pppctl", stdin, stdout, stderr);
[..]
..and probably more.
>How-To-Repeat:
Preparation:
dunstan@dunstan:(~)$ stty rows -1
dunstan@dunstan:(~)$ stty columns -1
dunstan@dunstan:(~)$ stty -a | head -1
speed 38400 baud; 65535 rows; 65535 columns;
dunstan@dunstan:(~)$ lpc
zsh: segmentation fault lpc
>Fix:
*Library* fix
el_init() should check result of term_init() and return NULL if term_init()
returned -1. Every application has to be corrected separately to properly
handle return value of el_init().
--- diff.0.el.c begins here ---
--- /usr/src/lib/libedit/el.c Tue Apr 26 00:51:51 2005
+++ src/lib/libedit/el.c Tue Apr 26 01:01:20 2005
@@ -74,14 +74,21 @@ el_init(const char *prog, FILE *fin, FIL
el->el_infd = fileno(fin);
el->el_outfile = fout;
el->el_errfile = ferr;
- el->el_prog = strdup(prog);
+ if ((el->el_prog = strdup(prog)) == NULL) {
+ el_free(el);
+ return (NULL);
+ }
/*
* Initialize all the modules. Order is important!!!
*/
el->el_flags = 0;
- (void) term_init(el);
+ if (term_init(el) == -1) {
+ free(el->el_prog);
+ el_free(el);
+ return (NULL);
+ }
(void) key_init(el);
(void) map_init(el);
if (tty_init(el) == -1)
--- diff.0.el.c ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200504252332.j3PNWGLG003380>