From 846e4600754dab3f0cb49edb4ad9e2b2b73d3f47 Mon Sep 17 00:00:00 2001 From: philhofer Date: Tue, 18 Dec 2018 21:02:24 -0800 Subject: [PATCH] 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. --- src/librc/librc-daemon.c | 4 ++-- src/librc/librc.c | 2 +- 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);