From 112dc1f135b5e960bba6188f8845382c5551ed8b Mon Sep 17 00:00:00 2001
From: Ingo Schwarze <schwarze@openbsd.org>
Date: Thu, 27 Jun 2019 15:07:30 +0000
Subject: Fix mandoc_normdate() and the way it is used. In the past, it could
 return NULL but the calling code wasn't prepared to handle that.  Make sure
 it always returns an allocated string. While here, simplify the code by
 handling the "quick" attribute inside mandoc_normdate() rather than at
 multiple callsites.

Triggered by deraadt@ pointing out
that snprintf(3) error handling was incomplete in time2a().
---
 man_validate.c  | 26 ++++++++++++--------------
 mandoc.c        | 11 ++++++++---
 mdoc_validate.c | 18 +++---------------
 3 files changed, 23 insertions(+), 32 deletions(-)

diff --git a/man_validate.c b/man_validate.c
index 55ac13a8..421a4843 100644
--- a/man_validate.c
+++ b/man_validate.c
@@ -185,8 +185,7 @@ check_root(CHKARGS)
 
 		man->meta.title = mandoc_strdup("");
 		man->meta.msec = mandoc_strdup("");
-		man->meta.date = man->quick ? mandoc_strdup("") :
-		    mandoc_normdate(man, NULL, n->line, n->pos);
+		man->meta.date = mandoc_normdate(man, NULL, n->line, n->pos);
 	}
 
 	if (man->meta.os_e &&
@@ -369,8 +368,8 @@ post_TH(CHKARGS)
 	/* ->TITLE<- MSEC DATE OS VOL */
 
 	n = n->child;
-	if (n && n->string) {
-		for (p = n->string; '\0' != *p; p++) {
+	if (n != NULL && n->string != NULL) {
+		for (p = n->string; *p != '\0'; p++) {
 			/* Only warn about this once... */
 			if (isalpha((unsigned char)*p) &&
 			    ! isupper((unsigned char)*p)) {
@@ -388,9 +387,9 @@ post_TH(CHKARGS)
 
 	/* TITLE ->MSEC<- DATE OS VOL */
 
-	if (n)
+	if (n != NULL)
 		n = n->next;
-	if (n && n->string)
+	if (n != NULL && n->string != NULL)
 		man->meta.msec = mandoc_strdup(n->string);
 	else {
 		man->meta.msec = mandoc_strdup("");
@@ -400,17 +399,16 @@ post_TH(CHKARGS)
 
 	/* TITLE MSEC ->DATE<- OS VOL */
 
-	if (n)
+	if (n != NULL)
 		n = n->next;
-	if (n && n->string && '\0' != n->string[0]) {
-		man->meta.date = man->quick ?
-		    mandoc_strdup(n->string) :
-		    mandoc_normdate(man, n->string, n->line, n->pos);
-	} else {
+	if (n != NULL && n->string != NULL && n->string[0] != '\0')
+		man->meta.date = mandoc_normdate(man,
+		    n->string, n->line, n->pos);
+	else {
 		man->meta.date = mandoc_strdup("");
 		mandoc_msg(MANDOCERR_DATE_MISSING,
-		    n ? n->line : nb->line,
-		    n ? n->pos : nb->pos, "TH");
+		    n == NULL ? nb->line : n->line,
+		    n == NULL ? nb->pos : n->pos, "TH");
 	}
 
 	/* TITLE MSEC DATE ->OS<- VOL */
diff --git a/mandoc.c b/mandoc.c
index 434d5d93..46cedc67 100644
--- a/mandoc.c
+++ b/mandoc.c
@@ -494,9 +494,10 @@ time2a(time_t t)
 	size_t		 ssz;
 	int		 isz;
 
+	buf = NULL;
 	tm = localtime(&t);
 	if (tm == NULL)
-		return NULL;
+		goto fail;
 
 	/*
 	 * Reserve space:
@@ -520,7 +521,8 @@ time2a(time_t t)
 	 * of looking at LC_TIME.
 	 */
 
-	if ((isz = snprintf(p, 4 + 1, "%d, ", tm->tm_mday)) == -1)
+	isz = snprintf(p, 4 + 1, "%d, ", tm->tm_mday);
+	if (isz < 0 || isz > 4)
 		goto fail;
 	p += isz;
 
@@ -530,7 +532,7 @@ time2a(time_t t)
 
 fail:
 	free(buf);
-	return NULL;
+	return mandoc_strdup("");
 }
 
 char *
@@ -539,6 +541,9 @@ mandoc_normdate(struct roff_man *man, char *in, int ln, int pos)
 	char		*cp;
 	time_t		 t;
 
+	if (man->quick)
+		return mandoc_strdup(in == NULL ? "" : in);
+
 	/* No date specified: use today's date. */
 
 	if (in == NULL || *in == '\0')
diff --git a/mdoc_validate.c b/mdoc_validate.c
index 14fbf828..ef2df798 100644
--- a/mdoc_validate.c
+++ b/mdoc_validate.c
@@ -1903,8 +1903,7 @@ post_root(POST_ARGS)
 	/* Add missing prologue data. */
 
 	if (mdoc->meta.date == NULL)
-		mdoc->meta.date = mdoc->quick ? mandoc_strdup("") :
-		    mandoc_normdate(mdoc, NULL, 0, 0);
+		mdoc->meta.date = mandoc_normdate(mdoc, NULL, 0, 0);
 
 	if (mdoc->meta.title == NULL) {
 		mandoc_msg(MANDOCERR_DT_NOTITLE, 0, 0, "EOF");
@@ -2519,21 +2518,10 @@ post_dd(POST_ARGS)
 		mandoc_msg(MANDOCERR_PROLOG_ORDER,
 		    n->line, n->pos, "Dd after Os");
 
-	if (n->child == NULL || n->child->string[0] == '\0') {
-		mdoc->meta.date = mdoc->quick ? mandoc_strdup("") :
-		    mandoc_normdate(mdoc, NULL, n->line, n->pos);
-		return;
-	}
-
 	datestr = NULL;
 	deroff(&datestr, n);
-	if (mdoc->quick)
-		mdoc->meta.date = datestr;
-	else {
-		mdoc->meta.date = mandoc_normdate(mdoc,
-		    datestr, n->line, n->pos);
-		free(datestr);
-	}
+	mdoc->meta.date = mandoc_normdate(mdoc, datestr, n->line, n->pos);
+	free(datestr);
 }
 
 static void
-- 
cgit