7076364 2001-09-08 14:56 -0400  /240 rader/ Greg A. Woods <woods@weird.com>
Sänt av: joel@lysator.liu.se
Importerad: 2001-09-10  08:15  av Brevbäraren
Extern mottagare: bind-workers@isc.org
Extern kopiemottagare: BUGTRAQ: Full Disclosure Security Mailing List <bugtraq@SecurityFocus.com>
Externa svar till: woods@planix.com
Mottagare: Bugtraq (import) <19133>
Ärende: PATCH to BIND-8.2.3 to get rid of the, unnecessary, and potentially dangerous fchown() calls
------------------------------------------------------------
From: woods@weird.com (Greg A. Woods)
To: bind-workers@isc.org
Cc: bugtraq@SecurityFocus.com (BUGTRAQ: Full Disclosure Security Mailing List)
Message-ID: <20010908185624.A2E99EC@proven.weird.com>

[[ note this posting is CC'ed to BUGTRAQ.  I know of no current
exploits in BIND-8.2.3, but even so since I'm also enclosing a fix
there may be quite a few people who might want to be able to fix
their own versions. ]]

The so-called "support" fix in change 999 of BIND-8.2.3 introduces
some unnecessary, and potentially very dangerous fchown() calls to
named.

The worst one is the one that leaves the pid-file writable by the
supposedly untrusted user named has been asked to run as.

The pid-file does not need to be owned, or indeed accessible in any
way, by the user named is running as.  If named is to be restarted
through any means but exec(), when it is running with low privileges,
then it must be restarted by root anyway!

One of the problems with leaving the pid-file writable by the
unprivileged user is of course that on many systems there are vendor
supplied (and sometimes home-grown) administrative control scripts
commonly used to manage daemon processes.  These scripts use the pid
stored in the pid-file to stop the daemon, trigger it to reload its
config, etc.  If the pid-file is writable by the user named is
running as then it is vulnerable should any remote exploit be found
for named.  This means a successful remote exploit of named could be
used to trick the sys-admin into killing arbitrary processes, and
indeed maybe even crashing the system.  Some of these scripts naively
put at least the first whole line, and sometimes even the entire
contents, from the pid-file on the command-line they use to invoke
kill.  The potential for danger is probably even higher for these
scripts.  [Note that asking the vendor to fix their scripts is silly
and pointless -- there's no way for the script to know intuitively
whether a PID has the "right" value because if they did then they
wouldn't need the pid-file in the first place, and indeed these
scripts rightfully assume that only a privileged user could have
created the pid-file in the first place!]

Another problem is that the pid-file may be located on a very
important file system, and even with low privileges a vulnerability
in named could allow the attacker to fill that file system.

There's also a call to change the ownership of any logging file (which
would only occur if named is explicitly configured to write to its own
log files instead of using syslog).  I don't know yet whether this
happens before named drops its privileges or not, but if so then the
fchown() results in leaving the log file writable by any successful
attacker!

The other calls to fchown() are probably just unnecessary and
pointless since they seem occur only after the file has just been
newly created, and if my quick analysis is correct only ever after
named has permanently given up root privileges too (though my
analysis of this aspect so far is rather incomplete).

If named is intentionally running as root (though I can't imagine why
anyone would still be doing that, except out of laziness) then these
calls will do nothing if the file did not previously exist, but if
the file did exist then these calls will change its ownership to be
root, which is potentially dangerous because although fchown() is
used no checks are made to ensure the file that was opened is indeed
freshly created and/or not tricked into pointing at some other
existing file by a symlink.  Yes normally anyone with any concern
about security will have pointed named into a private directory
that's intended to be writable only by the unprivileged user named is
normally expected to run as.  However that very same user is the one
which would be compromised by a remote vulnerability, and as such
there still no protection against a rogue symlink being created by a
successful attacker.

If named is not running as root (as it should not ever be!) then
these calls cannot likely ever do anything (since unlike the pid-file
call, all happen, as far as I can tell with a quick analysis, after
named has relinquished its privileges, so are pointless and
unnecessary (which is perhaps indicated implicitly by the way they
are not checked for failures).

Note the same problem occurs, but in a much more insidious way, in
BIND-9.  I do not yet have a patch to fix BIND-9 [I'm not running it
in production anywhere yet].

-- 
							Greg A. Woods

+1 416 218-0098      VE3TCP      <gwoods@acm.org>     <woods@robohack.ca>
Planix, Inc. <woods@planix.com>;   Secrets of the Weird <woods@weird.com>


Index: src/bin/named/ns_config.c
===================================================================
RCS file: /cvs/misc/bind8/src/bin/named/ns_config.c,v
retrieving revision 1.1.1.5
diff -c -r1.1.1.5 ns_config.c
*** src/bin/named/ns_config.c	31 Jan 2001 21:03:33 -0000	1.1.1.5
--- src/bin/named/ns_config.c	8 Sep 2001 03:40:31 -0000
***************
*** 1454,1460 ****
--- 1454,1462 ----
  		  S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
  	if (fd < 0)
  		return (NULL);
+ #if 0 /* ARGH!!!!  Making the pid file writable by user_id is a major security hole!!!! */
  	(void) fchown(fd, user_id, group_id);
+ #endif
  	stream = fdopen(fd, "w");
  	if (stream == NULL)
  		(void)close(fd);
Index: src/bin/named/ns_main.c
===================================================================
RCS file: /cvs/misc/bind8/src/bin/named/ns_main.c,v
retrieving revision 1.1.1.5
diff -c -r1.1.1.5 ns_main.c
*** src/bin/named/ns_main.c	31 Jan 2001 21:03:37 -0000	1.1.1.5
--- src/bin/named/ns_main.c	8 Sep 2001 03:29:07 -0000
***************
*** 621,627 ****
--- 621,629 ----
  			return;
  		case EBADF:
  		case ENOTSOCK:
+ #if 1	/* Note I didn't have this one listed for some reason....  -GAW */
  		case EFAULT:
+ #endif
  			/*
  			 * If one these happens, we're broken.
  			 */
Index: src/bin/named/ns_maint.c
===================================================================
RCS file: /cvs/misc/bind8/src/bin/named/ns_maint.c,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 ns_maint.c
*** src/bin/named/ns_maint.c	31 Jan 2001 21:03:37 -0000	1.1.1.4
--- src/bin/named/ns_maint.c	8 Sep 2001 18:30:52 -0000
***************
*** 667,673 ****
--- 667,675 ----
  				   name);
  			return(-1);
  		}
+ #if 0 /* ARGH!!!  this one's totally unnecessary given the file is guaranteed brand new!!! */
  		(void) fchown(tsig_fd, user_id, group_id);
+ #endif
  	}
  
  	memset(secret_buf, 0, sizeof(secret_buf));
Index: src/bin/named/ns_stats.c
===================================================================
RCS file: /cvs/misc/bind8/src/bin/named/ns_stats.c,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 ns_stats.c
*** src/bin/named/ns_stats.c	31 Jan 2001 21:03:41 -0000	1.1.1.4
--- src/bin/named/ns_stats.c	8 Sep 2001 03:49:45 -0000
***************
*** 122,128 ****
--- 122,130 ----
  			  server_options->stats_filename);
  	}
  	if (f != NULL) {
+ #if 0 /* ARGH!!!! */
  		(void) fchown(fileno(f), user_id, group_id);
+ #endif
  		fprintf(f, "+++ Host Statistics Cleared +++ (%ld) %s",
  			(long)timenow, checked_ctime(&timenow));
  		(void) my_fclose(f);
***************
*** 143,149 ****
--- 145,153 ----
  			  server_options->stats_filename);
  		return;
  	}
+ #if 0 /* ARGH!!!! */
  	(void) fchown(fileno(f), user_id, group_id);
+ #endif
  
  	fprintf(f, "+++ Statistics Dump +++ (%ld) %s",
  		(long)timenow, checked_ctime(&timenow));
***************
*** 170,176 ****
--- 174,182 ----
  			  server_options->memstats_filename);
  		return;
  	}
+ #if 0 /* ARGH!!!! */
  	(void) fchown(fileno(f), user_id, group_id);
+ #endif
  
  	fprintf(f, "+++ Memory Statistics Dump +++ (%ld) %s",
  		(long)timenow, checked_ctime(&timenow));
Index: src/bin/named/ns_update.c
===================================================================
RCS file: /cvs/misc/bind8/src/bin/named/ns_update.c,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 ns_update.c
*** src/bin/named/ns_update.c	31 Jan 2001 21:03:41 -0000	1.1.1.4
--- src/bin/named/ns_update.c	8 Sep 2001 03:50:10 -0000
***************
*** 145,151 ****
--- 145,153 ----
  			 strerror(errno));
  		return (NULL);
  	}
+ #if 0 /* ARGH!!!! */
  	(void) fchown(fileno(fp), user_id, group_id);
+ #endif
  	if (fseek(fp, 0L, SEEK_END) != 0) {
  		ns_error(ns_log_update, "can't fseek(%s, 0, SEEK_END)",
  			 zp->z_updatelog);
***************
*** 170,176 ****
--- 172,180 ----
  			 strerror(errno));
  		return (NULL);
  	}
+ #if 0 /* ARGH!!!! */
  	(void) fchown(fileno(fp), user_id, group_id);
+ #endif
  	if (fseek(fp, 0L, SEEK_END) != 0) {
  		ns_error(ns_log_update, "can't fseek(%s, 0, SEEK_END)",
  			 zp->z_ixfr_base);
Index: src/lib/isc/logging.c
===================================================================
RCS file: /cvs/misc/bind8/src/lib/isc/logging.c,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 logging.c
*** src/lib/isc/logging.c	31 Jan 2001 21:04:45 -0000	1.1.1.4
--- src/lib/isc/logging.c	8 Sep 2001 18:32:45 -0000
***************
*** 156,162 ****
--- 156,164 ----
  		chan->flags |= LOG_CHANNEL_BROKEN;
  		return (NULL);
  	}
+ #if 0 /* ARGH!!!  Don't leave the audit trail writable by the attacker!!! */
  	(void) fchown(fd, chan->out.file.owner, chan->out.file.group);
+ #endif
  
  	chan->out.file.stream = stream;
  	return (stream);
(7076364) /Greg A. Woods <woods@weird.com>/(Ombruten)