diff options
author | philhofer <phofer@umich.edu> | 2018-12-18 21:02:24 -0800 |
---|---|---|
committer | William Hubbs <w.d.hubbs@gmail.com> | 2018-12-27 11:28:27 -0600 |
commit | 846e4600754dab3f0cb49edb4ad9e2b2b73d3f47 (patch) | |
tree | 037e9c5f49a983a7d92bca78ebdd074087ecdbf0 | |
parent | a32b14bbb43e9888acaaea6f764fb8dcb34fb941 (diff) |
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.
-rw-r--r-- | src/librc/librc-daemon.c | 4 | ||||
-rw-r--r-- | src/librc/librc.c | 2 | ||||
-rw-r--r-- | src/rc/openrc-run.c | 2 |
3 files changed, 4 insertions, 4 deletions
diff --git a/src/librc/librc-daemon.c b/src/librc/librc-daemon.c index 2b0d9712..5a76c880 100644 --- a/src/librc/librc-daemon.c +++ b/src/librc/librc-daemon.c @@ -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'; } diff --git a/src/librc/librc.c b/src/librc/librc.c index c38695cc..1bdd9307 100644 --- a/src/librc/librc.c +++ b/src/librc/librc.c @@ -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); } diff --git a/src/rc/openrc-run.c b/src/rc/openrc-run.c index 229f5ed3..7649b04c 100644 --- a/src/rc/openrc-run.c +++ b/src/rc/openrc-run.c @@ -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); |