mdocml: Audit strlcpy(3)/strlcat(3) usage.

mdocml: Audit strlcpy(3)/strlcat(3) usage.

[schwarze]

Log Message:
-----------
Audit strlcpy(3)/strlcat(3) usage.

* Repair three instances of silent truncation, use asprintf(3).
* Change two instances of strlen(3)+malloc(3)+strlcpy(3)+strlcat(3)+...
to use asprintf(3) instead to make them less error prone.
* Cast the return value of four instances where the destination
buffer is known to be large enough to (void).
* Completely remove three useless instances of strlcpy(3)/strlcat(3).
* Mark two places in -Thtml with XXX that can cause information loss
and crashes but are not easy to fix, requiring design changes of 
some internal interfaces.
* The file mandocdb.c remains to be audited.

Modified Files:
--------------
    mdocml:
        TODO
        html.c
        man_html.c
        man_term.c
        mdoc_html.c
        mdoc_term.c
        mdoc_validate.c
        roff.c
        tbl_data.c

Revision Data
-------------
Index: man_html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_html.c,v
retrieving revision 1.94
retrieving revision 1.95
diff -Lman_html.c -Lman_html.c -u -p -r1.94 -r1.95
--- man_html.c
+++ man_html.c
@@ -301,15 +301,10 @@ a2width(const struct man_node *n, struct
 static void
 man_root_pre(MAN_ARGS)
 {
-	char		 b[BUFSIZ];
 	struct htmlpair	 tag[3];
 	struct tag	*t, *tt;
 	char		*title;
 
-	b[0] = 0;
-	if (man->vol)
-		(void)strlcat(b, man->vol, BUFSIZ);
-
 	assert(man->title);
 	assert(man->msec);
 	mandoc_asprintf(&title, "%s(%s)", man->title, man->msec);
@@ -335,7 +330,8 @@ man_root_pre(MAN_ARGS)
 	PAIR_CLASS_INIT(&tag[0], "head-vol");
 	PAIR_INIT(&tag[1], ATTR_ALIGN, "center");
 	print_otag(h, TAG_TD, 2, tag);
-	print_text(h, b);
+	if (NULL != man->vol)
+		print_text(h, man->vol);
 	print_stagq(h, tt);
 
 	PAIR_CLASS_INIT(&tag[0], "head-rtitle");
Index: html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/html.c,v
retrieving revision 1.156
retrieving revision 1.157
diff -Lhtml.c -Lhtml.c -u -p -r1.156 -r1.157
--- html.c
+++ html.c
@@ -657,6 +657,12 @@ void
 bufcat(struct html *h, const char *p)
 {
 
+	/*
+	 * XXX This is broken and not easy to fix.
+	 * When using the -Oincludes option, buffmt_includes()
+	 * may pass in strings overrunning BUFSIZ, causing a crash.
+	 */
+
 	h->buflen = strlcat(h->buf, p, BUFSIZ);
 	assert(h->buflen < BUFSIZ);
 }
Index: mdoc_html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_html.c,v
retrieving revision 1.189
retrieving revision 1.190
diff -Lmdoc_html.c -Lmdoc_html.c -u -p -r1.189 -r1.190
--- mdoc_html.c
+++ mdoc_html.c
@@ -515,18 +515,15 @@ mdoc_root_post(MDOC_ARGS)
 static int
 mdoc_root_pre(MDOC_ARGS)
 {
-	char		 b[BUFSIZ];
 	struct htmlpair	 tag[3];
 	struct tag	*t, *tt;
-	char		*title;
+	char		*volume, *title;
 
-	strlcpy(b, meta->vol, BUFSIZ);
-
-	if (meta->arch) {
-		strlcat(b, " (", BUFSIZ);
-		strlcat(b, meta->arch, BUFSIZ);
-		strlcat(b, ")", BUFSIZ);
-	}
+	if (NULL == meta->arch)
+		volume = mandoc_strdup(meta->vol);
+	else
+		mandoc_asprintf(&volume, "%s (%s)",
+		    meta->vol, meta->arch);
 
 	mandoc_asprintf(&title, "%s(%s)", meta->title, meta->msec);
 
@@ -551,7 +548,7 @@ mdoc_root_pre(MDOC_ARGS)
 	PAIR_CLASS_INIT(&tag[0], "head-vol");
 	PAIR_INIT(&tag[1], ATTR_ALIGN, "center");
 	print_otag(h, TAG_TD, 2, tag);
-	print_text(h, b);
+	print_text(h, volume);
 	print_stagq(h, tt);
 
 	PAIR_CLASS_INIT(&tag[0], "head-rtitle");
@@ -561,6 +558,7 @@ mdoc_root_pre(MDOC_ARGS)
 	print_tagq(h, t);
 
 	free(title);
+	free(volume);
 	return(1);
 }
 
@@ -993,8 +991,8 @@ mdoc_bl_pre(MDOC_ARGS)
 	PAIR_STYLE_INIT(&tag[0], h);
 
 	assert(lists[n->norm->Bl.type]);
-	strlcpy(buf, "list ", BUFSIZ);
-	strlcat(buf, lists[n->norm->Bl.type], BUFSIZ);
+	(void)strlcpy(buf, "list ", BUFSIZ);
+	(void)strlcat(buf, lists[n->norm->Bl.type], BUFSIZ);
 	PAIR_INIT(&tag[1], ATTR_CLASS, buf);
 
 	/* Set the block's left-hand margin. */
@@ -1363,6 +1361,15 @@ mdoc_fd_pre(MDOC_ARGS)
 
 	if (NULL != (n = n->next)) {
 		assert(MDOC_TEXT == n->type);
+
+		/*
+		 * XXX This is broken and not easy to fix.
+		 * When using -Oincludes, truncation may occur.
+		 * Dynamic allocation wouldn't help because
+		 * passing long strings to buffmt_includes()
+		 * does not work either.
+		 */
+
 		strlcpy(buf, '<' == *n->string || '"' == *n->string ?
 		    n->string + 1 : n->string, BUFSIZ);
 
@@ -1475,10 +1482,8 @@ mdoc_fn_pre(MDOC_ARGS)
 
 	t = print_otag(h, TAG_B, 1, tag);
 
-	if (sp) {
-		strlcpy(nbuf, sp, BUFSIZ);
-		print_text(h, nbuf);
-	}
+	if (sp)
+		print_text(h, sp);
 
 	print_tagq(h, t);
 
Index: roff.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/roff.c,v
retrieving revision 1.208
retrieving revision 1.209
diff -Lroff.c -Lroff.c -u -p -r1.208 -r1.209
--- roff.c
+++ roff.c
@@ -490,14 +490,13 @@ roff_res(struct roff *r, char **bufp, si
 {
 	char		 ubuf[24]; /* buffer to print the number */
 	const char	*start;	/* start of the string to process */
-	const char	*stesc;	/* start of an escape sequence ('\\') */
+	char		*stesc;	/* start of an escape sequence ('\\') */
 	const char	*stnam;	/* start of the name, after "[(*" */
 	const char	*cp;	/* end of the name, e.g. before ']' */
 	const char	*res;	/* the string to be substituted */
 	char		*nbuf;	/* new buffer to copy bufp to */
 	size_t		 maxl;  /* expected length of the escape name */
 	size_t		 naml;	/* actual length of the escape name */
-	size_t		 ressz;	/* size of the replacement string */
 	int		 expand_count;	/* to avoid infinite loops */
 	int		 npos;	/* position in numeric expression */
 	int		 irc;	/* return code from roff_evalnum() */
@@ -520,7 +519,7 @@ roff_res(struct roff *r, char **bufp, si
 				break;
 
 		if (0 == (stesc - cp) % 2) {
-			stesc = cp;
+			stesc = (char *)cp;
 			continue;
 		}
 
@@ -628,21 +627,17 @@ roff_res(struct roff *r, char **bufp, si
 			    ln, (int)(stesc - *bufp), NULL);
 			res = "";
 		}
-		ressz = strlen(res);
 
 		/* Replace the escape sequence by the string. */
 
-		*szp += ressz + 1;
-		nbuf = mandoc_malloc(*szp);
-
-		strlcpy(nbuf, *bufp, (size_t)(stesc - *bufp + 1));
-		strlcat(nbuf, res, *szp);
-		strlcat(nbuf, cp, *szp);
+		*stesc = '\0';
+		*szp = mandoc_asprintf(&nbuf, "%s%s%s",
+		    *bufp, res, cp) + 1;
 
 		/* Prepare for the next replacement. */
 
 		start = nbuf + pos;
-		stesc = nbuf + (stesc - *bufp) + ressz;
+		stesc = nbuf + (stesc - *bufp) + strlen(res);
 		free(*bufp);
 		*bufp = nbuf;
 	}
@@ -1990,14 +1985,9 @@ roff_userdef(ROFF_ARGS)
 			cp += 2;
 			continue;
 		}
-
-		*szp = strlen(n1) - 3 + strlen(arg[i]) + 1;
-		n2 = mandoc_malloc(*szp);
-
-		strlcpy(n2, n1, (size_t)(cp - n1 + 1));
-		strlcat(n2, arg[i], *szp);
-		strlcat(n2, cp + 3, *szp);
-
+		*cp = '\0';
+		*szp = mandoc_asprintf(&n2, "%s%s%s",
+		    n1, arg[i], cp + 3) + 1;
 		cp = n2 + (cp - n1);
 		free(n1);
 		n1 = n2;
Index: man_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_term.c,v
retrieving revision 1.147
retrieving revision 1.148
diff -Lman_term.c -Lman_term.c -u -p -r1.147 -r1.148
--- man_term.c
+++ man_term.c
@@ -1119,20 +1119,17 @@ print_man_foot(struct termp *p, const vo
 static void
 print_man_head(struct termp *p, const void *arg)
 {
-	char			 buf[BUFSIZ];
 	const struct man_meta	*meta;
+	const char		*volume;
 	char			*title;
-	size_t			 buflen, titlen;
+	size_t			 vollen, titlen;
 
 	meta = (const struct man_meta *)arg;
 	assert(meta->title);
 	assert(meta->msec);
 
-	if (meta->vol)
-		strlcpy(buf, meta->vol, BUFSIZ);
-	else
-		buf[0] = '\0';
-	buflen = term_strlen(p, buf);
+	volume = NULL == meta->vol ? "" : meta->vol;
+	vollen = term_strlen(p, volume);
 
 	/* Top left corner: manual title and section. */
 
@@ -1142,10 +1139,9 @@ print_man_head(struct termp *p, const vo
 	p->flags |= TERMP_NOBREAK | TERMP_NOSPACE;
 	p->trailspace = 1;
 	p->offset = 0;
-	p->rmargin = 2 * (titlen+1) + buflen < p->maxrmargin ?
-	    (p->maxrmargin -
-	     term_strlen(p, buf) + term_len(p, 1)) / 2 :
-	    p->maxrmargin - buflen;
+	p->rmargin = 2 * (titlen+1) + vollen < p->maxrmargin ?
+	    (p->maxrmargin - vollen + term_len(p, 1)) / 2 :
+	    p->maxrmargin - vollen;
 
 	term_word(p, title);
 	term_flushln(p);
@@ -1154,10 +1150,10 @@ print_man_head(struct termp *p, const vo
 
 	p->flags |= TERMP_NOSPACE;
 	p->offset = p->rmargin;
-	p->rmargin = p->offset + buflen + titlen < p->maxrmargin ?
+	p->rmargin = p->offset + vollen + titlen < p->maxrmargin ?
 	    p->maxrmargin - titlen : p->maxrmargin;
 
-	term_word(p, buf);
+	term_word(p, volume);
 	term_flushln(p);
 
 	/* Top right corner: title and section, again. */
Index: tbl_data.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/tbl_data.c,v
retrieving revision 1.30
retrieving revision 1.31
diff -Ltbl_data.c -Ltbl_data.c -u -p -r1.30 -r1.31
--- tbl_data.c
+++ tbl_data.c
@@ -167,8 +167,8 @@ tbl_cdata(struct tbl_node *tbl, int ln, 
 	if (dat->string) {
 		sz = strlen(p) + strlen(dat->string) + 2;
 		dat->string = mandoc_realloc(dat->string, sz);
-		strlcat(dat->string, " ", sz);
-		strlcat(dat->string, p, sz);
+		(void)strlcat(dat->string, " ", sz);
+		(void)strlcat(dat->string, p, sz);
 	} else
 		dat->string = mandoc_strdup(p);
 
Index: TODO
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/TODO,v
retrieving revision 1.168
retrieving revision 1.169
diff -LTODO -LTODO -u -p -r1.168 -r1.169
--- TODO
+++ TODO
@@ -7,7 +7,9 @@
 * crashes
 ************************************************************************
 
-None known.
+- The abort() in bufcat(), html.c, can be triggered via buffmt_includes()
+  by running -Thtml -Oincludes on a file containing a long .In argument.
+  Fixing this will probably require reworking the whole bufcat() concept.
 
 ************************************************************************
 * missing features
Index: mdoc_validate.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_validate.c,v
retrieving revision 1.212
retrieving revision 1.213
diff -Lmdoc_validate.c -Lmdoc_validate.c -u -p -r1.212 -r1.213
--- mdoc_validate.c
+++ mdoc_validate.c
@@ -1183,9 +1183,9 @@ post_defaults(POST_ARGS)
 static int
 post_at(POST_ARGS)
 {
-	const char	 *p, *q;
-	char		 *buf;
-	size_t		  sz;
+	struct mdoc_node	*n;
+	const char		*std_att;
+	char			*att;
 
 	/*
 	 * If we have a child, look it up in the standard keys.  If a
@@ -1193,27 +1193,18 @@ post_at(POST_ARGS)
 	 * prefix "AT&T UNIX " to the existing data.
 	 */
 
-	if (NULL == mdoc->last->child)
+	if (NULL == (n = mdoc->last->child))
 		return(1);
 
-	assert(MDOC_TEXT == mdoc->last->child->type);
-	p = mdoc_a2att(mdoc->last->child->string);
-
-	if (p) {
-		free(mdoc->last->child->string);
-		mdoc->last->child->string = mandoc_strdup(p);
-	} else {
+	assert(MDOC_TEXT == n->type);
+	if (NULL == (std_att = mdoc_a2att(n->string))) {
 		mdoc_nmsg(mdoc, mdoc->last, MANDOCERR_BADATT);
-		p = "AT&T UNIX ";
-		q = mdoc->last->child->string;
-		sz = strlen(p) + strlen(q) + 1;
-		buf = mandoc_malloc(sz);
-		strlcpy(buf, p, sz);
-		strlcat(buf, q, sz);
-		free(mdoc->last->child->string);
-		mdoc->last->child->string = buf;
-	}
+		mandoc_asprintf(&att, "AT&T UNIX %s", n->string);
+	} else
+		att = mandoc_strdup(std_att);
 
+	free(n->string);
+	n->string = att;
 	return(1);
 }
 
Index: mdoc_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_term.c,v
retrieving revision 1.266
retrieving revision 1.267
diff -Lmdoc_term.c -Lmdoc_term.c -u -p -r1.266 -r1.267
--- mdoc_term.c
+++ mdoc_term.c
@@ -442,10 +442,9 @@ print_mdoc_foot(struct termp *p, const v
 static void
 print_mdoc_head(struct termp *p, const void *arg)
 {
-	char			 buf[BUFSIZ];
 	const struct mdoc_meta	*meta;
-	char			*title;
-	size_t			 buflen, titlen;
+	char			*volume, *title;
+	size_t			 vollen, titlen;
 
 	meta = (const struct mdoc_meta *)arg;
 
@@ -466,14 +465,12 @@ print_mdoc_head(struct termp *p, const v
 	p->rmargin = p->maxrmargin;
 
 	assert(meta->vol);
-	strlcpy(buf, meta->vol, BUFSIZ);
-	buflen = term_strlen(p, buf);
-
-	if (meta->arch) {
-		strlcat(buf, " (", BUFSIZ);
-		strlcat(buf, meta->arch, BUFSIZ);
-		strlcat(buf, ")", BUFSIZ);
-	}
+	if (NULL == meta->arch)
+		volume = mandoc_strdup(meta->vol);
+	else
+		mandoc_asprintf(&volume, "%s (%s)",
+		    meta->vol, meta->arch);
+	vollen = term_strlen(p, volume);
 
 	mandoc_asprintf(&title, "%s(%s)", meta->title, meta->msec);
 	titlen = term_strlen(p, title);
@@ -481,20 +478,19 @@ print_mdoc_head(struct termp *p, const v
 	p->flags |= TERMP_NOBREAK | TERMP_NOSPACE;
 	p->trailspace = 1;
 	p->offset = 0;
-	p->rmargin = 2 * (titlen+1) + buflen < p->maxrmargin ?
-	    (p->maxrmargin -
-	     term_strlen(p, buf) + term_len(p, 1)) / 2 :
-	    p->maxrmargin - buflen;
+	p->rmargin = 2 * (titlen+1) + vollen < p->maxrmargin ?
+	    (p->maxrmargin - vollen + term_len(p, 1)) / 2 :
+	    p->maxrmargin - vollen;
 
 	term_word(p, title);
 	term_flushln(p);
 
 	p->flags |= TERMP_NOSPACE;
 	p->offset = p->rmargin;
-	p->rmargin = p->offset + buflen + titlen < p->maxrmargin ?
+	p->rmargin = p->offset + vollen + titlen < p->maxrmargin ?
 	    p->maxrmargin - titlen : p->maxrmargin;
 
-	term_word(p, buf);
+	term_word(p, volume);
 	term_flushln(p);
 
 	p->flags &= ~TERMP_NOBREAK;
@@ -511,6 +507,7 @@ print_mdoc_head(struct termp *p, const v
 	p->offset = 0;
 	p->rmargin = p->maxrmargin;
 	free(title);
+	free(volume);
 }
 
 static size_t
--
 To unsubscribe send an email to source+unsubscribe@mdocml.bsd.lv