>Number:         17933
>Category:       bin
>Synopsis:       minor overflow bug in comsat
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Aug 13 13:49:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     David A. Holland
>Release:        NetBSD 1.6E
>Organization:
   - David A. Holland / dholland@eecs.harvard.edu
>Environment:

System: NetBSD alicante 1.6E NetBSD 1.6E (ALICANTE) #3: Mon Aug 5 17:48:40 EDT 2002 dholland@alicante:/usr/src/sys/arch/i386/compile/ALICANTE i386
Architecture: i386
Machine: i386

>Description:

	comsat will zoom off the end of a buffer, as root, if utmp is
	very large.

	As far as I can tell it cannot be exploited usefully - it will
	always run off the end of the heap and dump core.

	(Also, you need write access to utmp to abuse it, so at worst
	it would be a path from group utmp to root... and I suspect
	there are still a lot of those that are much easier to exploit.)

	The problem is that it adds extra space to an off_t, then
	assigns it into a u_int without checking for integer overflow,
	then calls realloc, but casts the original off_t to int
	(promoted to size_t) to specify how much to read.

	The result is that you can make it (1) reallocate the buffer
	so it's very small, and then (2) attempt to read nearly 2^32
	bytes into that buffer.

	Note that this is not even an effective DoS, as if you crash
	comsat this way inetd will just spawn another one.

>How-To-Repeat:
	As above.

>Fix:

Patch follows.

It also corrects a minor article of pedanticism concerning snprintf's
return value a bit further down.

(Note: the patch uses INT_MAX rather than UINT_MAX because the off_t
is cast through int when calling read. This may be excessively
paranoid, but better to be safe.)

Index: comsat.c
===================================================================
RCS file: /cvsroot/basesrc/libexec/comsat/comsat.c,v
retrieving revision 1.23
diff -u -r1.23 comsat.c
--- comsat.c	2002/03/18 23:34:21	1.23
+++ comsat.c	2002/08/13 20:36:10
@@ -56,6 +56,7 @@
 #include <errno.h>
 #include <netdb.h>
 #include <paths.h>
+#include <limits.h>
 #include <pwd.h>
 #include <signal.h>
 #include <stdio.h>
@@ -170,6 +171,11 @@
 	(void)fstat(uf, &statbf);
 	if (statbf.st_mtime > utmpmtime) {
 		utmpmtime = statbf.st_mtime;
+		/* avoid integer overflow */
+		if (statbf.st_size > INT_MAX - 10 * sizeof(struct utmp)) {
+			syslog(LOG_ALERT, "utmp file excessively large");
+			exit(1);
+		}
 		if (statbf.st_size > utmpsize) {
 			utmpsize = statbf.st_size + 10 * sizeof(struct utmp);
 			if ((utmp = realloc(utmp, utmpsize)) == NULL) {
@@ -207,7 +213,7 @@
 		char maildir[128];
 		int l = snprintf(maildir, sizeof(maildir), ":%s/%s",
 				 _PATH_MAILDIR, name);
-		if (l > sizeof(maildir) || strcmp(maildir, fn) != 0)
+		if (l >= sizeof(maildir) || strcmp(maildir, fn) != 0)
 			return;
 	}
 	while (--utp >= utmp)

>Release-Note:
>Audit-Trail:
>Unformatted: