Subject: bin/17933: minor overflow bug in comsat
To: None <>
From: David Holland <>
List: netbsd-bugs
Date: 08/13/2002 16:48:17
>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
>Originator:     David A. Holland
>Release:        NetBSD 1.6E
   - David A. Holland /

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


	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.

	As above.


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)
 	while (--utp >= utmp)