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.
Upstream maintainer, who appears to be <papowell>, has been notified.
Whoa, this is what I gets for going to company meetings. Trying to fix.
Okay, this looks fixed, one line no less. Passing it to QA.
Watch out - turns out there's more than one format string cock-up. Details to follow in 2 mins..
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); } } }
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
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.
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.
Great, Great. Well, I've been off net since that last update, so will get this in today.
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)
Ok, so I logged bug #18253, covering "to listen or not to listen, that is the question"
*** Bug 22346 has been marked as a duplicate of this bug. ***