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. :)
This commit is contained in:
FRIGN 2015-03-17 11:24:49 +01:00
parent a76d4943b5
commit 93fd817536
10 changed files with 43 additions and 28 deletions

6
find.c
View File

@ -977,10 +977,10 @@ find(char *path, Findhist *hist)
if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
continue; continue;
p = pathbuf + strlcpy(pathbuf, path, pathcap); p = pathbuf + estrlcpy(pathbuf, path, pathcap);
if (*--p != '/') if (*--p != '/')
strlcat(pathbuf, "/", pathcap); estrlcat(pathbuf, "/", pathcap);
strlcat(pathbuf, de->d_name, pathcap); estrlcat(pathbuf, de->d_name, pathcap);
find(pathbuf, &cur); find(pathbuf, &cur);
} }
closedir(dir); /* check return value? */ closedir(dir); /* check return value? */

View File

@ -66,12 +66,10 @@ recurse(const char *path, void *data, struct recursor *r)
} }
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, "..")) if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue; continue;
if (strlcpy(subpath, path, PATH_MAX) >= PATH_MAX) estrlcpy(subpath, path, sizeof(subpath));
eprintf("strlcpy: path too long\n"); if (path[strlen(path) - 1] != '/')
if (path[strlen(path) - 1] != '/' && strlcat(subpath, "/", PATH_MAX) >= PATH_MAX) estrlcat(subpath, "/", sizeof(subpath));
eprintf("strlcat: path too long\n"); estrlcat(subpath, d->d_name, sizeof(subpath));
if (strlcat(subpath, d->d_name, PATH_MAX) >= PATH_MAX)
eprintf("strlcat: path too long\n");
if ((r->flags & SAMEDEV) && statf(subpath, &dst) < 0) { if ((r->flags & SAMEDEV) && statf(subpath, &dst) < 0) {
if (errno != ENOENT) { if (errno != ENOENT) {
weprintf("%s %s:", statf_name, subpath); weprintf("%s %s:", statf_name, subpath);

View File

@ -50,3 +50,14 @@ strlcat(char *dst, const char *src, size_t siz)
*d = '\0'; *d = '\0';
return(dlen + (s - src)); /* count does not include NUL */ 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;
}

View File

@ -46,3 +46,14 @@ strlcpy(char *dst, const char *src, size_t siz)
} }
return(s - src - 1); /* count does not include NUL */ 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;
}

View File

@ -80,9 +80,9 @@ main(int argc, char *argv[])
sz += argc; sz += argc;
buf = ecalloc(1, sz); buf = ecalloc(1, sz);
for (i = 0; i < argc; i++) { for (i = 0; i < argc; i++) {
strlcat(buf, argv[i], sz); estrlcat(buf, argv[i], sz);
if (i + 1 < argc) if (i + 1 < argc)
strlcat(buf, " ", sz); estrlcat(buf, " ", sz);
} }
syslog(priority, "%s", buf); syslog(priority, "%s", buf);
} }

View File

@ -39,19 +39,14 @@ main(int argc, char *argv[])
if ((p = getenv("TMPDIR"))) if ((p = getenv("TMPDIR")))
tmpdir = p; tmpdir = p;
if (strlcpy(tmp, template, sizeof(tmp)) >= sizeof(tmp)) estrlcpy(tmp, template, sizeof(tmp));
eprintf("path too long\n");
p = dirname(tmp); p = dirname(tmp);
if (p[0] != '.') { if (p[0] != '.') {
if (strlcpy(path, template, sizeof(path)) >= sizeof(path)) estrlcpy(path, template, sizeof(path));
eprintf("path too long\n");
} else { } else {
if (strlcpy(path, tmpdir, sizeof(path)) >= sizeof(path)) estrlcpy(path, tmpdir, sizeof(path));
eprintf("path too long\n"); estrlcat(path, "/", sizeof(path));
if (strlcat(path, "/", sizeof(path)) >= sizeof(path)) estrlcat(path, template, sizeof(path));
eprintf("path too long\n");
if (strlcat(path, template, sizeof(path)) >= sizeof(path))
eprintf("path too long\n");
} }
if (dflag) { if (dflag) {

View File

@ -56,8 +56,7 @@ main(int argc, char *argv[])
arg[2] = '\0'; arg[2] = '\0';
} else } else
arg[0] = '\0'; arg[0] = '\0';
if (strlcat(arg, argv[0], PATH_MAX) >= PATH_MAX) estrlcat(arg, argv[0], sizeof(arg));
eprintf("path too long\n");
while ((p = strchr(p, '/'))) { while ((p = strchr(p, '/'))) {
*p = '\0'; *p = '\0';
if (!realpath(arg, b)) { if (!realpath(arg, b)) {
@ -75,8 +74,7 @@ mdone:
/* drop the extra '/' on root */ /* drop the extra '/' on root */
lp += (argv[0][0] == '/' && lp += (argv[0][0] == '/' &&
lp - arg == 1); lp - arg == 1);
if (strlcat(b, lp, PATH_MAX) >= PATH_MAX) estrlcat(b, lp, sizeof(arg));
eprintf("path too long\n");
} }
} }
break; break;

4
sed.c
View File

@ -329,7 +329,7 @@ strnacat(String *dst, char *src, size_t n)
resize((void **)&dst->str, &dst->cap, 1, len * 2, NULL); resize((void **)&dst->str, &dst->cap, 1, len * 2, NULL);
if (new) if (new)
*dst->str = '\0'; *dst->str = '\0';
strlcat(dst->str, src, len); estrlcat(dst->str, src, len);
} }
void void
@ -352,7 +352,7 @@ strnacpy(String *dst, char *src, size_t n)
len = strlen(dst->str) + MIN(n, len) + 1; len = strlen(dst->str) + MIN(n, len) + 1;
if (dst->cap < len) if (dst->cap < len)
resize((void **)&dst->str, &dst->cap, 1, len * 2, NULL); resize((void **)&dst->str, &dst->cap, 1, len * 2, NULL);
strlcpy(dst->str, src, len); estrlcpy(dst->str, src, len);
} }
void void

View File

@ -109,7 +109,7 @@ main(int argc, char *argv[])
plen = strlen(prefix); plen = strlen(prefix);
if (plen + slen > NAME_MAX) if (plen + slen > NAME_MAX)
eprintf("names cannot exceed %d bytes\n", 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 (file && strcmp(file, "-") != 0) {
if (!(in = fopen(file, "r"))) if (!(in = fopen(file, "r")))

2
util.h
View File

@ -48,8 +48,10 @@ char *strcasestr(const char *, const char *);
#undef strlcat #undef strlcat
size_t strlcat(char *, const char *, size_t); size_t strlcat(char *, const char *, size_t);
size_t estrlcat(char *, const char *, size_t);
#undef strlcpy #undef strlcpy
size_t strlcpy(char *, const char *, size_t); size_t strlcpy(char *, const char *, size_t);
size_t estrlcpy(char *, const char *, size_t);
#undef strsep #undef strsep
char *strsep(char **, const char *); char *strsep(char **, const char *);