From nobody Tue Oct 22 02:16:58 1996 Received: (from nobody@localhost) by freefall.freebsd.org (8.7.5/8.7.3) id CAA16358; Tue, 22 Oct 1996 02:16:58 -0700 (PDT) Message-Id: <199610220916.CAA16358@freefall.freebsd.org> Date: Tue, 22 Oct 1996 02:16:58 -0700 (PDT) From: tqbf@enteract.com To: freebsd-gnats-submit@freebsd.org Subject: On systems with setuid 'lpr' and defined printers, lpr breaks root X-Send-Pr-Version: www-1.0 >Number: 1863 >Category: bin >Synopsis: On systems with setuid 'lpr' and defined printers, lpr breaks root >Confidential: no >Severity: critical >Priority: high >Responsible: freebsd-bugs >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Oct 22 02:20:02 PDT 1996 >Closed-Date: Wed Nov 27 19:33:50 PST 1996 >Last-Modified: Wed Nov 27 19:34:38 PST 1996 >Originator: Thomas Ptacek >Release: Unresolved as of 2.2-Current >Organization: EnterAct, L.L.C. >Environment: FreeBSD adam 2.1-STABLE FreeBSD 2.1-STABLE #0: Mon Sep 9 03:07:45 CDT 1996 tqbf@adam:/home1/src/sys/compile/ADAMSTOMP i386 >Description: lpr contains a routine called 'card()', which takes an input string a single character described by an int. The routine copies the input string into a temporary buffer stored on the stack, prepended by the supplied character. No bounds checking is done during the copy, and the card() routine is called with a pointer obtained directly from getopt, causing a stack overflow. >How-To-Repeat: On systems with a defined printer: lpr -P -C `rootshellcode` where "rootshellcode" outputs a stream of characters containing return addresses pointing further into the buffer, and 8086 opcodes that will call execve() with "/bin/sh" as an argument. >Fix: card() keeps track of the length of the string as it copies it, and the copy takes place in a while loop. Check the incremented length of the string against the size of the temporary buffer, and break the copy as soon as the length is greater than the size of the buffer. >Release-Note: >Audit-Trail: From: Marc Slemko To: tqbf@enteract.com Cc: freebsd-gnats-submit@FreeBSD.ORG Subject: Re: bin/1863: On systems with setuid 'lpr' and defined printers, lpr breaks root Date: Tue, 22 Oct 1996 09:49:36 -0600 (MDT) Below is an excerpt from a diff between the current FreeBSD and the current OpenBSD lpr.c that shows how this problem is fixed in the OpenBSD source. There are also some other attempts at security improvements in the OpenBSD lpr code; they should be looked at to see if they are valid and, if so, imported. *************** *** 471,477 **** register int len = 2; *p1++ = c; ! while ((c = *p2++) != '\0') { *p1++ = (c == '\n') ? ' ' : c; len++; } --- 505,511 ---- register int len = 2; *p1++ = c; ! while ((c = *p2++) != '\0' && len < sizeof(buf)) { *p1++ = (c == '\n') ? ' ' : c; len++; } From: roberto@keltia.freenix.fr (Ollivier Robert) To: tqbf@enteract.com Cc: freebsd-gnats-submit@freebsd.org Subject: Re: bin/1863: On systems with setuid 'lpr' and defined printers, lpr breaks root Date: Tue, 22 Oct 1996 18:01:43 +0200 According to tqbf@enteract.com: > >Fix: > card() keeps track of the length of the string as it > copies it, and the copy takes place in a while loop. Check > the incremented length of the string against the size of > the temporary buffer, and break the copy as soon as the length > is greater than the size of the buffer. Here is a fix which truncate the input string if longer than BUFSIZ. I did not consider it worth dynamic allocation because the strings are supposed to be short in the control file. I've changed a sprint into snprintf while I was here. The lpr/* code is full of fixed buffers on the stack. Many of them can't be exploited unless your /etc/printcap is hacked then you already ahve a problem :-) Thanks for the report. Index: common_source/startdaemon.c =================================================================== RCS file: /spare/FreeBSD-current/src/usr.sbin/lpr/common_source/startdaemon.c,v retrieving revision 1.2 diff -u -2 -r1.2 startdaemon.c --- startdaemon.c 1996/05/09 22:44:00 1.2 +++ startdaemon.c 1996/10/22 15:44:12 @@ -79,5 +79,5 @@ return(0); } - (void) sprintf(buf, "\1%s\n", printer); + (void) snprintf(buf, sizeof buf, "\1%s\n", printer); n = strlen(buf); if (write(s, buf, n) != n) { Index: lpr/lpr.c =================================================================== RCS file: /spare/FreeBSD-current/src/usr.sbin/lpr/lpr/lpr.c,v retrieving revision 1.7 diff -u -2 -r1.7 lpr.c --- lpr.c 1996/05/11 19:00:55 1.7 +++ lpr.c 1996/10/22 15:28:52 @@ -470,6 +470,11 @@ register char *p1 = buf; register int len = 2; + register int ilen = strlen (p2); *p1++ = c; + if (ilen > BUFSIZ) { /* avoir trashing the stack and get root */ + ilen = BUFSIZ; + p2[ilen - 1] = '\0'; + } while ((c = *p2++) != '\0') { *p1++ = (c == '\n') ? ' ' : c; -- Ollivier ROBERT -=- The daemon is FREE! -=- roberto@keltia.freenix.fr FreeBSD keltia.freenix.fr 2.2-CURRENT #25: Tue Oct 15 21:13:57 MET DST 1996 State-Changed-From-To: open->closed State-Changed-By: fenner State-Changed-When: Wed Nov 27 19:33:50 PST 1996 State-Changed-Why: Fixed on October 27 by imp >Unformatted: