Bug 17756 - Critical security hole in LPRng, remote root
Critical security hole in LPRng, remote root
Status: CLOSED ERRATA
Product: Red Hat Linux
Classification: Retired
Component: LPRng (Show other bugs)
7.1
i386 Linux
high Severity medium
: ---
: ---
Assigned To: Crutcher Dunnavant
: Security
: 22346 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2000-09-20 18:48 EDT by Chris Evans
Modified: 2008-05-01 11:37 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2000-10-02 12:48:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Chris Evans 2000-09-20 18:48:45 EDT
NOTE:
In a default full install, "lpd" is running and listening on the
network. That really isn't so good for security. It should be considered
whether or
not print serving is a sufficiently common requirement to listen on the
network (with no access controls) by default.
.
LPRng contains a format string bug in a syslog() wrapper:
LPRng-3.6.22/src/common/errormsg.c:~145
...
syslog(kind, msg);
...
Unfortunately, syslog() is called for any line of network input which isn't
understood. So,

telnet my.dialup.ip printer

Trying x.x.x.x...
Connected to my.dialup.ip (x.x.x.x).
Escape character is '^]'.
%s
Connection closed by foreign host.

<sound of switching to another console>

...
[pid  2634] --- SIGSEGV (Segmentation fault) ---
...
Not good.
Comment 1 Chris Evans 2000-09-20 19:15:18 EDT
Upstream maintainer, who appears to be <papowell@astart.com>, has been notified.
Comment 2 Crutcher Dunnavant 2000-09-25 09:57:02 EDT
Whoa, this is what I gets for going to company meetings.
Trying to fix.
Comment 3 Crutcher Dunnavant 2000-09-25 10:54:24 EDT
Okay, this looks fixed, one line no less. Passing it to QA.
Comment 4 Chris Evans 2000-09-25 11:05:52 EDT
Watch out - turns out there's more than one format string cock-up.
Details to follow in 2 mins..
Comment 5 Chris Evans 2000-09-25 11:17:50 EDT
Oh - actually, this security patch from Olaf Kirch patches some dodgy looking sprintf() calls.
I'll paste it in here
--- LPRng-3.5.3/src/AUTHENTICATE/setupauth.c.overflows  Mon Sep 25 12:06:22 2000
+++ LPRng-3.5.3/src/AUTHENTICATE/setupauth.c    Mon Sep 25 12:06:31 2000
@@ -176,8 +176,9 @@
     return (cp);
 }
 
 int test_argv_cnt;
 char *test_argv[20];
 char buffer[1024];
 int count;
 
@@ -211,7 +211,7 @@
        struct passwd *pw;
        char line[256];
        char envs[256];
-       char *s, *end;
+       char *s, *end, *value;
        int len;
 
        test_argv_cnt = 0;
@@ -220,45 +220,42 @@
                fprintf( stderr, "user '%s' does not exist\n", user );
                exit(1);
        }
-       sprintf(line, "USER=%s", pw->pw_name);
+       snprintf(line, sizeof(line), "USER=%s", pw->pw_name);
        add_env( line );
 
-       sprintf(line, "LOGNAME=%s", pw->pw_name);
+       snprintf(line, sizeof(line), "LOGNAME=%s", pw->pw_name);
        add_env( line );
 
-       sprintf( line, "HOME=%s", pw->pw_dir);
+       snprintf(line, sizeof(line), "HOME=%s", pw->pw_dir);
        add_env( line );
 
-       sprintf( line, "LOGDIR=%s", pw->pw_dir);
+       snprintf(line, sizeof(line), "LOGDIR=%s", pw->pw_dir);
        add_env( line );
 
        add_env( "SHELL=/bin/sh" );
 
-       sprintf( line, "IFS= \t");
+       snprintf(line, sizeof(line), "IFS= \t");
        add_env( line );
 
        s = getenv( "TZ" );
        if( s ){
-               sprintf( line, "TZ=%s", s );
+               snprintf(line, sizeof(line), "TZ=%s", s );
                add_env( line );
        }
 
-       if( pass_env ){
+       if( pass_env  && strlen(pass_env) < sizeof(envs) ){
                strcpy( envs, pass_env );
                for( s = envs; s && *s; s = end ){
                        while( isspace( *s ) ) ++s;
                        end = strpbrk( s, " \t,;" );
                        if( end ) *end++ = 0;
                        /*fprintf(stderr,"looking for '%s'\n",s ); */
-                       if( strlen(s) ){
-                               strcpy( line, s );
-                               if( (s = getenv( line )) && strlen(s) != 0 ){
-                                       /*fprintf(stderr,"found '%s'\n",s );*/
-                                       len = strlen( line );
-                                       sprintf( line+len, "=%s", s );
-                                       /*fprintf(stderr,"adding '%s'\n",line ); */
-                                       add_env( line );
-                               }
+                       if( strlen(s)
+                        && (value = getenv(s))
+                        && strlen(value) ){
+                               snprintf(line, sizeof(line),
+                                       "%s=%s", s, value);
+                               add_env(line);
                        }
                }
        }
Comment 6 Chris Evans 2000-09-25 11:24:30 EDT
And some more info :-)
When I mailled the author to say I'd found a format string bug, he replied and said:
---
Right.  I have found quite a few others and plugged them as well.
---

This sounds quite concerning. If there are more format string issues, I'm guessing
you'll want to fix them all at once rather than have to issue more than one update.
I have asked the author for a patch fixing all these format string bugs, but a reply
has not been forthcoming yet.
Perhaps these bugs reside in the *print* functions, e.g.
fprintf(stderr, str);
That's a bug and it's quite common (but not as common as *syslog abuse)
In the absence of a reply from the author, we'll have to scan for these extra
format bugs ourselves. *sigh*. I'll have a look when I get home. Any help in
the meantime would be cool
Comment 7 Matthew Kirkwood 2000-09-25 11:53:13 EDT
I'd like this to be fixed "properly".  Basically, code issues aside, the hole
should not have been exploitable by default because:

1. The lpd should not have been listening by default, and,

2. It should not have read a single byte from the network before checking that
the remote host was allowed to print.

Clearly we need to get errata out pretty sharp, but it would be nice if such
basic precautions could make the default setup safer while people try to improve
the code.
Comment 8 Crutcher Dunnavant 2000-10-01 11:44:29 EDT
Oops, I ran this monday, forgot to update this bug.
There is an errata about this, but I agree that LPRng should check network
connections before accepting any data. Will look into that aspect.

We are looking at a full rewrite of printer configuration, and I will keep this
in mind.
Comment 9 Crutcher Dunnavant 2000-10-02 12:48:19 EDT
Great, Great.
Well, I've been off net since that last update, so will get this in today.
Comment 10 Crutcher Dunnavant 2000-10-02 13:40:06 EDT
Well, no, actually. This setupauth.c patch dosn't apply to the latest LPRng, in
the 3.6.24 release, which I updated to in the security errata, so I am
re-resolving this, as everything is more or less fixed (less being the issue of
whether we start lpd by default, but not for here)
Comment 11 Chris Evans 2000-10-03 14:56:28 EDT
Ok, so I logged bug #18253, covering "to listen or not to listen, that is the
question"
Comment 12 Daniel Roesen 2000-12-15 11:13:19 EST
*** Bug 22346 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.