FW: stack overflow in tzload

[Olson, Arthur David (NIH/NCI) [E]]

I'm forwarding this message from Ted Unangst, who is not on the time zone mailing list. Those of you who are on the list, please direct replies appropriately.

				--ado

-----Original Message-----
From: Ted Unangst [mailto:ted.unangst at gmail.com] 
Sent: Monday, November 22, 2010 6:22
To: tz at lecserver.nci.nih.gov
Subject: stack overflow in tzload

Stack overflow as in there's not enough stack.  I ran into this problem 
running a program on openbsd that was using a fairly small thread size.  I 
switched to using malloc.

Index: localtime.c
===================================================================
RCS file: /home/tedu/cvs/src/lib/libc/time/localtime.c,v
retrieving revision 1.35
diff -u -r1.35 localtime.c
--- localtime.c	23 Aug 2010 22:35:34 -0000	1.35
+++ localtime.c	20 Nov 2010 23:32:09 -0000
@@ -340,7 +340,7 @@
 		char		buf[2 * sizeof(struct tzhead) +
 					2 * sizeof *sp +
 					4 * TZ_MAX_TIMES];
-	} u;
+	} *u;
 
 	sp->goback = sp->goahead = FALSE;
 	if (name != NULL && issetugid() != 0)
@@ -383,28 +383,33 @@
 		if ((fid = open(name, OPEN_MODE)) == -1)
 			return -1;
 	}
-	nread = read(fid, u.buf, sizeof u.buf);
-	if (close(fid) < 0 || nread <= 0)
+	u = malloc(sizeof(*u));
+	if (!u) {
+		close(fid);
 		return -1;
+	}
+	nread = read(fid, u->buf, sizeof u->buf);
+	if (close(fid) < 0 || nread <= 0)
+		goto bad;
 	for (stored = 4; stored <= 8; stored *= 2) {
 		int		ttisstdcnt;
 		int		ttisgmtcnt;
 
-		ttisstdcnt = (int) detzcode(u.tzhead.tzh_ttisstdcnt);
-		ttisgmtcnt = (int) detzcode(u.tzhead.tzh_ttisgmtcnt);
-		sp->leapcnt = (int) detzcode(u.tzhead.tzh_leapcnt);
-		sp->timecnt = (int) detzcode(u.tzhead.tzh_timecnt);
-		sp->typecnt = (int) detzcode(u.tzhead.tzh_typecnt);
-		sp->charcnt = (int) detzcode(u.tzhead.tzh_charcnt);
-		p = u.tzhead.tzh_charcnt + sizeof u.tzhead.tzh_charcnt;
+		ttisstdcnt = (int) detzcode(u->tzhead.tzh_ttisstdcnt);
+		ttisgmtcnt = (int) detzcode(u->tzhead.tzh_ttisgmtcnt);
+		sp->leapcnt = (int) detzcode(u->tzhead.tzh_leapcnt);
+		sp->timecnt = (int) detzcode(u->tzhead.tzh_timecnt);
+		sp->typecnt = (int) detzcode(u->tzhead.tzh_typecnt);
+		sp->charcnt = (int) detzcode(u->tzhead.tzh_charcnt);
+		p = u->tzhead.tzh_charcnt + sizeof u->tzhead.tzh_charcnt;
 		if (sp->leapcnt < 0 || sp->leapcnt > TZ_MAX_LEAPS ||
 			sp->typecnt <= 0 || sp->typecnt > TZ_MAX_TYPES ||
 			sp->timecnt < 0 || sp->timecnt > TZ_MAX_TIMES ||
 			sp->charcnt < 0 || sp->charcnt > TZ_MAX_CHARS ||
 			(ttisstdcnt != sp->typecnt && ttisstdcnt != 0) ||
 			(ttisgmtcnt != sp->typecnt && ttisgmtcnt != 0))
-				return -1;
-		if (nread - (p - u.buf) <
+				goto bad;
+		if (nread - (p - u->buf) <
 			sp->timecnt * stored +		/* ats */
 			sp->timecnt +			/* types */
 			sp->typecnt * 6 +		/* ttinfos */
@@ -412,7 +417,7 @@
 			sp->leapcnt * (stored + 4) +	/* lsinfos */
 			ttisstdcnt +			/* ttisstds */
 			ttisgmtcnt)			/* ttisgmts */
-				return -1;
+				goto bad;
 		for (i = 0; i < sp->timecnt; ++i) {
 			sp->ats[i] = (stored == 4) ?
 				detzcode(p) : detzcode64(p);
@@ -421,7 +426,7 @@
 		for (i = 0; i < sp->timecnt; ++i) {
 			sp->types[i] = (unsigned char) *p++;
 			if (sp->types[i] >= sp->typecnt)
-				return -1;
+				goto bad;
 		}
 		for (i = 0; i < sp->typecnt; ++i) {
 			register struct ttinfo *	ttisp;
@@ -431,11 +436,11 @@
 			p += 4;
 			ttisp->tt_isdst = (unsigned char) *p++;
 			if (ttisp->tt_isdst != 0 && ttisp->tt_isdst != 1)
-				return -1;
+				goto bad;
 			ttisp->tt_abbrind = (unsigned char) *p++;
 			if (ttisp->tt_abbrind < 0 ||
 				ttisp->tt_abbrind > sp->charcnt)
-					return -1;
+					goto bad;
 		}
 		for (i = 0; i < sp->charcnt; ++i)
 			sp->chars[i] = *p++;
@@ -460,7 +465,7 @@
 				ttisp->tt_ttisstd = *p++;
 				if (ttisp->tt_ttisstd != TRUE &&
 					ttisp->tt_ttisstd != FALSE)
-						return -1;
+						goto bad;
 			}
 		}
 		for (i = 0; i < sp->typecnt; ++i) {
@@ -473,7 +478,7 @@
 				ttisp->tt_ttisgmt = *p++;
 				if (ttisp->tt_ttisgmt != TRUE &&
 					ttisp->tt_ttisgmt != FALSE)
-						return -1;
+						goto bad;
 			}
 		}
 		/*
@@ -506,11 +511,11 @@
 		/*
 		** If this is an old file, we're done.
 		*/
-		if (u.tzhead.tzh_version[0] == '\0')
+		if (u->tzhead.tzh_version[0] == '\0')
 			break;
-		nread -= p - u.buf;
+		nread -= p - u->buf;
 		for (i = 0; i < nread; ++i)
-			u.buf[i] = p[i];
+			u->buf[i] = p[i];
 		/*
 		** If this is a narrow integer time_t system, we're done.
 		*/
@@ -518,13 +523,13 @@
 			break;
 	}
 	if (doextend && nread > 2 &&
-		u.buf[0] == '\n' && u.buf[nread - 1] == '\n' &&
+		u->buf[0] == '\n' && u->buf[nread - 1] == '\n' &&
 		sp->typecnt + 2 <= TZ_MAX_TYPES) {
 			struct state	ts;
 			register int	result;
 
-			u.buf[nread - 1] = '\0';
-			result = tzparse(&u.buf[1], &ts, FALSE);
+			u->buf[nread - 1] = '\0';
+			result = tzparse(&u->buf[1], &ts, FALSE);
 			if (result == 0 && ts.typecnt == 2 &&
 				sp->charcnt + ts.charcnt <= TZ_MAX_CHARS) {
 					for (i = 0; i < 2; ++i)
@@ -568,7 +573,11 @@
 					break;
 		}
 	}
+	free(u);
 	return 0;
+bad:
+	free(u);
+	return -1;
 }
 
 static int