fix potential out-of-bounds reads

readlink(3) does not nul-terminate the result it sticks
into the supplied buffer. Consequently, the code

  rc = readlink(path, buf, sizeof(buf));

does not necessarily produce a C string.

The code in rc_find_pid() produces some C strings this way
and passes them to strlen() and strcmp(), which can lead
to an out-of-bounds read.

In this case, since the code already takes care to
zero-initialize the buffers before passing them
to readlink(3), only allow sizeof(buf)-1 bytes to
be returned.

(While fixing this issue, I fixed two other locations that
used the same problematic pattern.)

This fixes #270.
This commit is contained in:
philhofer 2018-12-18 21:02:24 -08:00 committed by William Hubbs
parent a32b14bbb4
commit 846e460075
3 changed files with 4 additions and 4 deletions

View File

@ -147,7 +147,7 @@ rc_find_pids(const char *exec, const char *const *argv, uid_t uid, pid_t pid)
memset(my_ns, 0, sizeof(my_ns));
memset(proc_ns, 0, sizeof(proc_ns));
if (exists("/proc/self/ns/pid")) {
rc = readlink("/proc/self/ns/pid", my_ns, sizeof(my_ns));
rc = readlink("/proc/self/ns/pid", my_ns, sizeof(my_ns)-1);
if (rc <= 0)
my_ns[0] = '\0';
}
@ -161,7 +161,7 @@ rc_find_pids(const char *exec, const char *const *argv, uid_t uid, pid_t pid)
continue;
xasprintf(&buffer, "/proc/%d/ns/pid", p);
if (exists(buffer)) {
rc = readlink(buffer, proc_ns, sizeof(proc_ns));
rc = readlink(buffer, proc_ns, sizeof(proc_ns)-1);
if (rc <= 0)
proc_ns[0] = '\0';
}

View File

@ -558,7 +558,7 @@ rc_service_resolve(const char *service)
if (*file) {
memset(buffer, 0, sizeof(buffer));
r = readlink(file, buffer, sizeof(buffer));
r = readlink(file, buffer, sizeof(buffer)-1);
if (r > 0)
return xstrdup(buffer);
}

View File

@ -1152,7 +1152,7 @@ int main(int argc, char **argv)
}
lnk = xmalloc(4096);
memset(lnk, 0, 4096);
if (readlink(argv[1], lnk, 4096)) {
if (readlink(argv[1], lnk, 4096-1)) {
dir = dirname(path);
if (strchr(lnk, '/')) {
save = xstrdup(dir);