aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorIngo Schwarze <schwarze@openbsd.org>2014-04-23 16:08:33 +0000
committerIngo Schwarze <schwarze@openbsd.org>2014-04-23 16:08:33 +0000
commite1bb09be17d41880b391cdcbe9d968f8964928ff (patch)
treeeeb097d0331817dc1ea9f7b52f2930635bda4387
parent1f555653eb03957679b5749f7ac779126c3c2627 (diff)
downloadmandoc-e1bb09be17d41880b391cdcbe9d968f8964928ff.tar.gz
mandoc-e1bb09be17d41880b391cdcbe9d968f8964928ff.tar.zst
mandoc-e1bb09be17d41880b391cdcbe9d968f8964928ff.zip
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.
-rw-r--r--TODO6
-rw-r--r--html.c8
-rw-r--r--man_html.c10
-rw-r--r--man_term.c24
-rw-r--r--mdoc_html.c39
-rw-r--r--mdoc_term.c33
-rw-r--r--mdoc_validate.c33
-rw-r--r--roff.c30
-rw-r--r--tbl_data.c6
9 files changed, 86 insertions, 103 deletions
diff --git a/TODO b/TODO
index e619f272..2d4f2622 100644
--- a/TODO
+++ b/TODO
@@ -1,13 +1,15 @@
************************************************************************
* Official mandoc TODO.
-* $Id: TODO,v 1.168 2014/03/30 19:47:48 schwarze Exp $
+* $Id: TODO,v 1.169 2014/04/23 16:08:33 schwarze Exp $
************************************************************************
************************************************************************
* 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
diff --git a/html.c b/html.c
index 7319de66..b8a4c445 100644
--- a/html.c
+++ b/html.c
@@ -1,4 +1,4 @@
-/* $Id: html.c,v 1.156 2014/04/20 16:46:04 schwarze Exp $ */
+/* $Id: html.c,v 1.157 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2011, 2012, 2013, 2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -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);
}
diff --git a/man_html.c b/man_html.c
index 092c4c21..5df9e469 100644
--- a/man_html.c
+++ b/man_html.c
@@ -1,4 +1,4 @@
-/* $Id: man_html.c,v 1.94 2014/04/20 20:18:12 schwarze Exp $ */
+/* $Id: man_html.c,v 1.95 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2013, 2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -301,15 +301,10 @@ a2width(const struct man_node *n, struct roffsu *su)
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");
diff --git a/man_term.c b/man_term.c
index f3fc4114..e308f6a3 100644
--- a/man_term.c
+++ b/man_term.c
@@ -1,4 +1,4 @@
-/* $Id: man_term.c,v 1.147 2014/04/20 20:18:12 schwarze Exp $ */
+/* $Id: man_term.c,v 1.148 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2010-2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -1119,20 +1119,17 @@ print_man_foot(struct termp *p, const void *arg)
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 void *arg)
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 void *arg)
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. */
diff --git a/mdoc_html.c b/mdoc_html.c
index ac1e43b4..2dc15c53 100644
--- a/mdoc_html.c
+++ b/mdoc_html.c
@@ -1,4 +1,4 @@
-/* $Id: mdoc_html.c,v 1.189 2014/04/20 20:18:12 schwarze Exp $ */
+/* $Id: mdoc_html.c,v 1.190 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -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);
diff --git a/mdoc_term.c b/mdoc_term.c
index fac902bc..93bb05ea 100644
--- a/mdoc_term.c
+++ b/mdoc_term.c
@@ -1,4 +1,4 @@
-/* $Id: mdoc_term.c,v 1.266 2014/04/20 20:18:12 schwarze Exp $ */
+/* $Id: mdoc_term.c,v 1.267 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2010, 2012, 2013, 2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -442,10 +442,9 @@ print_mdoc_foot(struct termp *p, const void *arg)
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 void *arg)
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 void *arg)
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 void *arg)
p->offset = 0;
p->rmargin = p->maxrmargin;
free(title);
+ free(volume);
}
static size_t
diff --git a/mdoc_validate.c b/mdoc_validate.c
index 55573fa8..8c394328 100644
--- a/mdoc_validate.c
+++ b/mdoc_validate.c
@@ -1,4 +1,4 @@
-/* $Id: mdoc_validate.c,v 1.212 2014/04/20 20:48:53 schwarze Exp $ */
+/* $Id: mdoc_validate.c,v 1.213 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2010-2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -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);
}
diff --git a/roff.c b/roff.c
index fee1a8c1..07480a12 100644
--- a/roff.c
+++ b/roff.c
@@ -1,4 +1,4 @@
-/* $Id: roff.c,v 1.208 2014/04/20 19:40:13 schwarze Exp $ */
+/* $Id: roff.c,v 1.209 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2010, 2011, 2012 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2010-2014 Ingo Schwarze <schwarze@openbsd.org>
@@ -490,14 +490,13 @@ roff_res(struct roff *r, char **bufp, size_t *szp, int ln, int pos)
{
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, size_t *szp, int ln, int pos)
break;
if (0 == (stesc - cp) % 2) {
- stesc = cp;
+ stesc = (char *)cp;
continue;
}
@@ -628,21 +627,17 @@ roff_res(struct roff *r, char **bufp, size_t *szp, int ln, int pos)
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;
diff --git a/tbl_data.c b/tbl_data.c
index 2a362e5c..1db24a61 100644
--- a/tbl_data.c
+++ b/tbl_data.c
@@ -1,4 +1,4 @@
-/* $Id: tbl_data.c,v 1.30 2014/04/20 16:46:05 schwarze Exp $ */
+/* $Id: tbl_data.c,v 1.31 2014/04/23 16:08:33 schwarze Exp $ */
/*
* Copyright (c) 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
* Copyright (c) 2011 Ingo Schwarze <schwarze@openbsd.org>
@@ -167,8 +167,8 @@ tbl_cdata(struct tbl_node *tbl, int ln, const char *p)
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);