From b407caa0b5b60c5bb9fc7733886b884617ba16fe Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 17 May 2004 07:52:46 +0000 Subject: [PATCH] pullup: 1661. [cleanup] indiscriminate use strlcat/strlcpy make auditing harder. --- lib/bind/irs/dns_ho.c | 54 +++++++++++++--------------- lib/bind/irs/dns_nw.c | 9 ++--- lib/bind/irs/gen_gr.c | 74 +++++++++++++++++++++----------------- lib/bind/irs/getaddrinfo.c | 6 +--- lib/bind/irs/hesiod.c | 35 +++++------------- 5 files changed, 78 insertions(+), 100 deletions(-) diff --git a/lib/bind/irs/dns_ho.c b/lib/bind/irs/dns_ho.c index 46f2ca580f..cac33b9ddf 100644 --- a/lib/bind/irs/dns_ho.c +++ b/lib/bind/irs/dns_ho.c @@ -52,7 +52,7 @@ /* BIND Id: gethnamaddr.c,v 8.15 1996/05/22 04:56:30 vixie Exp $ */ #if defined(LIBC_SCCS) && !defined(lint) -static const char rcsid[] = "$Id: dns_ho.c,v 1.14 2004/03/18 02:57:58 marka Exp $"; +static const char rcsid[] = "$Id: dns_ho.c,v 1.15 2004/05/17 07:52:46 marka Exp $"; #endif /* LIBC_SCCS and not lint */ /* Imports. */ @@ -414,38 +414,44 @@ ho_byaddr(struct irs_ho *this, const void *addr, int len, int af) break; case AF_INET6: if (q->action != RESTGT_IGNORE) { + const char *nibsuff = res_get_nibblesuffix(pvt->res); qp = q->qname; for (n = IN6ADDRSZ - 1; n >= 0; n--) { i = SPRINTF((qp, "%x.%x.", uaddr[n] & 0xf, (uaddr[n] >> 4) & 0xf)); - if (i < 0) + if (i != 4) abort(); qp += i; } -#ifdef HAVE_STRLCAT - strlcat(q->qname, res_get_nibblesuffix(pvt->res), - sizeof(q->qname)); -#else - strcpy(qp, res_get_nibblesuffix(pvt->res)); -#endif + if (strlen(q->qname) + strlen(nibsuff) + 1 > + sizeof q->qname) { + errno = ENAMETOOLONG; + RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL); + hp = NULL; + goto cleanup; + } + strcpy(qp, nibsuff); /* (checked) */ } if (q2->action != RESTGT_IGNORE) { + const char *nibsuff2 = res_get_nibblesuffix2(pvt->res); qp = q2->qname; for (n = IN6ADDRSZ - 1; n >= 0; n--) { i = SPRINTF((qp, "%x.%x.", uaddr[n] & 0xf, (uaddr[n] >> 4) & 0xf)); - if (i < 0) + if (i != 4) abort(); qp += i; } -#ifdef HAVE_STRLCAT - strlcat(q->qname, res_get_nibblesuffix2(pvt->res), - sizeof(q->qname)); -#else - strcpy(qp, res_get_nibblesuffix2(pvt->res)); -#endif + if ((qp - q->qname) + strlen(nibsuff2) + 1 > + sizeof q->qname){ + errno = ENAMETOOLONG; + RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL); + hp = NULL; + goto cleanup; + } + strcpy(qp, nibsuff2); /* (checked) */ } break; default: @@ -820,11 +826,7 @@ gethostans(struct irs_ho *this, had_error++; continue; } -#ifdef HAVE_STRLCPY - strlcpy(bp, tbuf, ep - bp); -#else - strcpy(bp, tbuf); -#endif + strcpy(bp, tbuf); /* (checked) */ pvt->host.h_name = bp; hname = bp; bp += n; @@ -856,11 +858,7 @@ gethostans(struct irs_ho *this, had_error++; continue; } -#ifdef HAVE_STRLCPY - strlcpy(bp, tbuf, ep - bp); -#else - strcpy(bp, tbuf); -#endif + strcpy(bp, tbuf); /* (checked) */ tname = bp; bp += n; continue; @@ -996,11 +994,7 @@ gethostans(struct irs_ho *this, n = strlen(qname) + 1; /* for the \0 */ if (n > (ep - bp) || n >= MAXHOSTNAMELEN) goto no_recovery; -#ifdef HAVE_STRLCPY - strlcpy(bp, qname, ep - bp); -#else - strcpy(bp, qname); -#endif + strcpy(bp, qname); /* (checked) */ pvt->host.h_name = bp; bp += n; } diff --git a/lib/bind/irs/dns_nw.c b/lib/bind/irs/dns_nw.c index 79a5aefcf0..8368b747f6 100644 --- a/lib/bind/irs/dns_nw.c +++ b/lib/bind/irs/dns_nw.c @@ -16,7 +16,7 @@ */ #if defined(LIBC_SCCS) && !defined(lint) -static const char rcsid[] = "$Id: dns_nw.c,v 1.9 2004/03/18 02:57:58 marka Exp $"; +static const char rcsid[] = "$Id: dns_nw.c,v 1.10 2004/05/17 07:52:46 marka Exp $"; #endif /* LIBC_SCCS and not lint */ /* Imports. */ @@ -349,12 +349,7 @@ get1101answer(struct irs_nw *this, RES_SET_H_ERRNO(pvt->res, NO_RECOVERY); return (NULL); } -#ifdef HAVE_STRLCPY - strlcpy(bp, name, ep - bp); - pvt->net.n_name = bp; -#else - pvt->net.n_name = strcpy(bp, name); -#endif + pvt->net.n_name = strcpy(bp, name); /* (checked) */ bp += n; } break; diff --git a/lib/bind/irs/gen_gr.c b/lib/bind/irs/gen_gr.c index a32bdd26d6..34fec98ad8 100644 --- a/lib/bind/irs/gen_gr.c +++ b/lib/bind/irs/gen_gr.c @@ -16,7 +16,7 @@ */ #if !defined(LINT) && !defined(CODECENTER) -static const char rcsid[] = "$Id: gen_gr.c,v 1.6 2004/03/09 06:30:00 marka Exp $"; +static const char rcsid[] = "$Id: gen_gr.c,v 1.7 2004/05/17 07:52:46 marka Exp $"; #endif /* Imports */ @@ -83,7 +83,7 @@ static void gr_res_set(struct irs_gr *, struct __res_state *, void (*)(void *)); -static void grmerge(struct irs_gr *gr, const struct group *src, +static int grmerge(struct irs_gr *gr, const struct group *src, int preserve); static int countvec(char **vec); @@ -92,6 +92,10 @@ static int countnew(char **old, char **new); static size_t sizenew(char **old, char **new); static int newgid(int, gid_t *, gid_t); +/* Macros */ + +#define FREE_IF(x) do { if ((x) != NULL) { free(x); (x) = NULL; } } while (0) + /* Public */ struct irs_gr * @@ -171,7 +175,8 @@ gr_byname(struct irs_gr *this, const char *name) { gr = rule->inst->gr; tval = (*gr->byname)(gr, name); if (tval) { - grmerge(this, tval, dirty++); + if (!grmerge(this, tval, dirty++)) + return (NULL); if (!(rule->flags & IRS_MERGE)) break; } else { @@ -197,7 +202,8 @@ gr_bygid(struct irs_gr *this, gid_t gid) { gr = rule->inst->gr; tval = (*gr->bygid)(gr, gid); if (tval) { - grmerge(this, tval, dirty++); + if (!grmerge(this, tval, dirty++)) + return (NULL); if (!(rule->flags & IRS_MERGE)) break; } else { @@ -321,7 +327,7 @@ gr_res_set(struct irs_gr *this, struct __res_state *res, /* Private. */ -static void +static int grmerge(struct irs_gr *this, const struct group *src, int preserve) { struct pvt *pvt = (struct pvt *)this->private; char *cp, **m, **p, *oldmembuf, *ep; @@ -332,9 +338,9 @@ grmerge(struct irs_gr *this, const struct group *src, int preserve) { pvt->group.gr_gid = src->gr_gid; if (pvt->nmemb < 1) { m = malloc(sizeof *m); - if (!m) { + if (m == NULL) { /* No harm done, no work done. */ - return; + return (0); } pvt->group.gr_mem = m; pvt->nmemb = 1; @@ -351,9 +357,9 @@ grmerge(struct irs_gr *this, const struct group *src, int preserve) { n = ndst + nnew + 1; if ((size_t)n > pvt->nmemb) { m = realloc(pvt->group.gr_mem, n * sizeof *m); - if (!m) { + if (m == NULL) { /* No harm done, no work done. */ - return; + return (0); } pvt->group.gr_mem = m; pvt->nmemb = n; @@ -371,13 +377,13 @@ grmerge(struct irs_gr *this, const struct group *src, int preserve) { } if (n == 0) { /* No work to do. */ - return; + return (1); } used = preserve ? pvt->membufsize : 0; cp = malloc(used + n); - if (!cp) { + if (cp == NULL) { /* No harm done, no work done. */ - return; + return (0); } ep = cp + used + n; if (used != 0) @@ -401,12 +407,13 @@ grmerge(struct irs_gr *this, const struct group *src, int preserve) { if (isnew(pvt->group.gr_mem, *m)) { *p++ = cp; *p = NULL; -#ifdef HAVE_STRLCPY - strlcpy(cp, *m, ep - cp); -#else - strcpy(cp, *m); -#endif - cp += strlen(cp) + 1; + n = strlen(*m) + 1; + if (n > ep - cp) { + FREE_IF(oldmembuf); + return (0); + } + strcpy(cp, *m); /* (checked) */ + cp += n; } if (preserve) { pvt->group.gr_name = pvt->membuf + @@ -415,23 +422,26 @@ grmerge(struct irs_gr *this, const struct group *src, int preserve) { (pvt->group.gr_passwd - oldmembuf); } else { pvt->group.gr_name = cp; -#ifdef HAVE_STRLCPY - strlcpy(cp, src->gr_name, ep - cp); -#else - strcpy(cp, src->gr_name); -#endif - cp += strlen(src->gr_name) + 1; + n = strlen(src->gr_name) + 1; + if (n > ep - cp) { + FREE_IF(oldmembuf); + return (0); + } + strcpy(cp, src->gr_name); /* (checked) */ + cp += n; + pvt->group.gr_passwd = cp; -#ifdef HAVE_STRLCPY - strlcpy(cp, src->gr_passwd, ep - cp); -#else - strcpy(cp, src->gr_passwd); -#endif - cp += strlen(src->gr_passwd) + 1; + n = strlen(src->gr_passwd) + 1; + if (n > ep - cp) { + FREE_IF(oldmembuf); + return (0); + } + strcpy(cp, src->gr_passwd); /* (checked) */ + cp += n; } - if (oldmembuf != NULL) - free(oldmembuf); + FREE_IF(oldmembuf); INSIST(cp >= pvt->membuf && cp <= &pvt->membuf[pvt->membufsize]); + return (1); } static int diff --git a/lib/bind/irs/getaddrinfo.c b/lib/bind/irs/getaddrinfo.c index 89db519fcf..31a45367e7 100644 --- a/lib/bind/irs/getaddrinfo.c +++ b/lib/bind/irs/getaddrinfo.c @@ -937,11 +937,7 @@ copy_ai(pai) free(ai); return NULL; } -#ifdef HAVE_STRLCPY - strlcpy(ai->ai_canonname, pai->ai_canonname, l); -#else - strncpy(ai->ai_canonname, pai->ai_canonname, l); -#endif + strcpy(ai->ai_canonname, pai->ai_canonname); /* (checked) */ } else { /* just to make sure */ ai->ai_canonname = NULL; diff --git a/lib/bind/irs/hesiod.c b/lib/bind/irs/hesiod.c index 714a48dd3a..b6ffbfaf8f 100644 --- a/lib/bind/irs/hesiod.c +++ b/lib/bind/irs/hesiod.c @@ -1,5 +1,5 @@ #if defined(LIBC_SCCS) && !defined(lint) -static const char rcsid[] = "$Id: hesiod.c,v 1.4 2004/03/18 02:57:59 marka Exp $"; +static const char rcsid[] = "$Id: hesiod.c,v 1.5 2004/05/17 07:52:46 marka Exp $"; #endif /* @@ -92,19 +92,14 @@ hesiod_init(void **context) { /* * Use compiled in defaults. */ - ctx->LHS = malloc(strlen(DEF_LHS)+1); - ctx->RHS = malloc(strlen(DEF_RHS)+1); - if (ctx->LHS == 0 || ctx->RHS == 0) { + ctx->LHS = malloc(strlen(DEF_LHS) + 1); + ctx->RHS = malloc(strlen(DEF_RHS) + 1); + if (ctx->LHS == NULL || ctx->RHS == NULL) { errno = ENOMEM; goto cleanup; } -#ifdef HAVE_STRLCPY - strlcpy(ctx->LHS, DEF_LHS, strlen(DEF_LHS) + 1); - strlcpy(ctx->RHS, DEF_RHS, strlen(DEF_RHS) + 1); -#else - strcpy(ctx->LHS, DEF_LHS); - strcpy(ctx->RHS, DEF_RHS); -#endif + strcpy(ctx->LHS, DEF_LHS); /* (checked) */ + strcpy(ctx->RHS, DEF_RHS); /* (checked) */ #else goto cleanup; #endif @@ -123,22 +118,10 @@ hesiod_init(void **context) { goto cleanup; } if (cp[0] == '.') { -#ifdef HAVE_STRLCPY - strlcpy(ctx->RHS, cp, RHSlen); -#else - strcpy(ctx->RHS, cp); -#endif + strcpy(ctx->RHS, cp); /* (checked) */ } else { -#ifdef HAVE_STRLCPY - strlcpy(ctx->RHS, ".", RHSlen); -#else - strcpy(ctx->RHS, "."); -#endif -#ifdef HAVE_STRLCAT - strlcat(ctx->RHS, cp, RHSlen); -#else - strcat(ctx->RHS, cp); -#endif + strcpy(ctx->RHS, "."); /* (checked) */ + strcat(ctx->RHS, cp); /* (checked) */ } }