From 93fd8175361337dbbf6557d7cc6542790bb5dd26 Mon Sep 17 00:00:00 2001 From: FRIGN Date: Tue, 17 Mar 2015 11:24:49 +0100 Subject: [PATCH] Add estrlcat() and estrlcpy() It has become a common idiom in sbase to check strlcat() and strlcpy() using if (strl{cat, cpy}(dst, src, siz) >= siz) eprintf("path too long\n"); However, this was not carried out consistently and to this very day, some tools employed unchecked calls to these functions, effectively allowing silent truncations to happen, which in turn may lead to security issues. To finally put an end to this, the e*-functions detect truncation automatically and the caller can lean back and enjoy coding without trouble. :) --- find.c | 6 +++--- libutil/recurse.c | 10 ++++------ libutil/strlcat.c | 11 +++++++++++ libutil/strlcpy.c | 11 +++++++++++ logger.c | 4 ++-- mktemp.c | 15 +++++---------- readlink.c | 6 ++---- sed.c | 4 ++-- split.c | 2 +- util.h | 2 ++ 10 files changed, 43 insertions(+), 28 deletions(-) diff --git a/find.c b/find.c index 9b876dc..9a427a2 100644 --- a/find.c +++ b/find.c @@ -977,10 +977,10 @@ find(char *path, Findhist *hist) if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) continue; - p = pathbuf + strlcpy(pathbuf, path, pathcap); + p = pathbuf + estrlcpy(pathbuf, path, pathcap); if (*--p != '/') - strlcat(pathbuf, "/", pathcap); - strlcat(pathbuf, de->d_name, pathcap); + estrlcat(pathbuf, "/", pathcap); + estrlcat(pathbuf, de->d_name, pathcap); find(pathbuf, &cur); } closedir(dir); /* check return value? */ diff --git a/libutil/recurse.c b/libutil/recurse.c index 1637d0c..c4e8043 100644 --- a/libutil/recurse.c +++ b/libutil/recurse.c @@ -66,12 +66,10 @@ recurse(const char *path, void *data, struct recursor *r) } if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, "..")) continue; - if (strlcpy(subpath, path, PATH_MAX) >= PATH_MAX) - eprintf("strlcpy: path too long\n"); - if (path[strlen(path) - 1] != '/' && strlcat(subpath, "/", PATH_MAX) >= PATH_MAX) - eprintf("strlcat: path too long\n"); - if (strlcat(subpath, d->d_name, PATH_MAX) >= PATH_MAX) - eprintf("strlcat: path too long\n"); + estrlcpy(subpath, path, sizeof(subpath)); + if (path[strlen(path) - 1] != '/') + estrlcat(subpath, "/", sizeof(subpath)); + estrlcat(subpath, d->d_name, sizeof(subpath)); if ((r->flags & SAMEDEV) && statf(subpath, &dst) < 0) { if (errno != ENOENT) { weprintf("%s %s:", statf_name, subpath); diff --git a/libutil/strlcat.c b/libutil/strlcat.c index 9e2d251..bf263b8 100644 --- a/libutil/strlcat.c +++ b/libutil/strlcat.c @@ -50,3 +50,14 @@ strlcat(char *dst, const char *src, size_t siz) *d = '\0'; return(dlen + (s - src)); /* count does not include NUL */ } + +size_t +estrlcat(char *dst, const char *src, size_t siz) +{ + size_t ret; + + if ((ret = strlcat(dst, src, siz)) >= siz) + eprintf("strlcat: input string too long\n"); + + return ret; +} diff --git a/libutil/strlcpy.c b/libutil/strlcpy.c index 388b426..44b618a 100644 --- a/libutil/strlcpy.c +++ b/libutil/strlcpy.c @@ -46,3 +46,14 @@ strlcpy(char *dst, const char *src, size_t siz) } return(s - src - 1); /* count does not include NUL */ } + +size_t +estrlcpy(char *dst, const char *src, size_t siz) +{ + size_t ret; + + if ((ret = strlcpy(dst, src, siz)) >= siz) + eprintf("strlcpy: input string too long\n"); + + return ret; +} diff --git a/logger.c b/logger.c index 0f8a763..80dcdc7 100644 --- a/logger.c +++ b/logger.c @@ -80,9 +80,9 @@ main(int argc, char *argv[]) sz += argc; buf = ecalloc(1, sz); for (i = 0; i < argc; i++) { - strlcat(buf, argv[i], sz); + estrlcat(buf, argv[i], sz); if (i + 1 < argc) - strlcat(buf, " ", sz); + estrlcat(buf, " ", sz); } syslog(priority, "%s", buf); } diff --git a/mktemp.c b/mktemp.c index 8dbb210..b4ecaf1 100644 --- a/mktemp.c +++ b/mktemp.c @@ -39,19 +39,14 @@ main(int argc, char *argv[]) if ((p = getenv("TMPDIR"))) tmpdir = p; - if (strlcpy(tmp, template, sizeof(tmp)) >= sizeof(tmp)) - eprintf("path too long\n"); + estrlcpy(tmp, template, sizeof(tmp)); p = dirname(tmp); if (p[0] != '.') { - if (strlcpy(path, template, sizeof(path)) >= sizeof(path)) - eprintf("path too long\n"); + estrlcpy(path, template, sizeof(path)); } else { - if (strlcpy(path, tmpdir, sizeof(path)) >= sizeof(path)) - eprintf("path too long\n"); - if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) - eprintf("path too long\n"); - if (strlcat(path, template, sizeof(path)) >= sizeof(path)) - eprintf("path too long\n"); + estrlcpy(path, tmpdir, sizeof(path)); + estrlcat(path, "/", sizeof(path)); + estrlcat(path, template, sizeof(path)); } if (dflag) { diff --git a/readlink.c b/readlink.c index e19b6c9..9c6c789 100644 --- a/readlink.c +++ b/readlink.c @@ -56,8 +56,7 @@ main(int argc, char *argv[]) arg[2] = '\0'; } else arg[0] = '\0'; - if (strlcat(arg, argv[0], PATH_MAX) >= PATH_MAX) - eprintf("path too long\n"); + estrlcat(arg, argv[0], sizeof(arg)); while ((p = strchr(p, '/'))) { *p = '\0'; if (!realpath(arg, b)) { @@ -75,8 +74,7 @@ mdone: /* drop the extra '/' on root */ lp += (argv[0][0] == '/' && lp - arg == 1); - if (strlcat(b, lp, PATH_MAX) >= PATH_MAX) - eprintf("path too long\n"); + estrlcat(b, lp, sizeof(arg)); } } break; diff --git a/sed.c b/sed.c index f341947..04f847f 100644 --- a/sed.c +++ b/sed.c @@ -329,7 +329,7 @@ strnacat(String *dst, char *src, size_t n) resize((void **)&dst->str, &dst->cap, 1, len * 2, NULL); if (new) *dst->str = '\0'; - strlcat(dst->str, src, len); + estrlcat(dst->str, src, len); } void @@ -352,7 +352,7 @@ strnacpy(String *dst, char *src, size_t n) len = strlen(dst->str) + MIN(n, len) + 1; if (dst->cap < len) resize((void **)&dst->str, &dst->cap, 1, len * 2, NULL); - strlcpy(dst->str, src, len); + estrlcpy(dst->str, src, len); } void diff --git a/split.c b/split.c index 1401bfd..b68635f 100644 --- a/split.c +++ b/split.c @@ -109,7 +109,7 @@ main(int argc, char *argv[]) plen = strlen(prefix); if (plen + slen > NAME_MAX) eprintf("names cannot exceed %d bytes\n", NAME_MAX); - strlcpy(name, prefix, sizeof(name)); + estrlcpy(name, prefix, sizeof(name)); if (file && strcmp(file, "-") != 0) { if (!(in = fopen(file, "r"))) diff --git a/util.h b/util.h index bccf57f..39d7d0d 100644 --- a/util.h +++ b/util.h @@ -48,8 +48,10 @@ char *strcasestr(const char *, const char *); #undef strlcat size_t strlcat(char *, const char *, size_t); +size_t estrlcat(char *, const char *, size_t); #undef strlcpy size_t strlcpy(char *, const char *, size_t); +size_t estrlcpy(char *, const char *, size_t); #undef strsep char *strsep(char **, const char *);