Skip to content

Commit

Permalink
src/librc/librc-daemon.c: fix buffer overrun in pid_is_argv
Browse files Browse the repository at this point in the history
The contents of /proc/<pid>/cmdline are read into
a stack buffer using

  bytes = read(fd, buffer, sizeof(buffer));

followed by appending a null terminator to the buffer with

  buffer[bytes] = '\0';

If bytes == sizeof(buffer), then this write is out-of-bounds.

Refactor the code to use rc_getfile instead, since PATH_MAX
is not the maximum size of /proc/<pid>/cmdline. (I hit this
issue in practice while compiling Linux; it tripped the
stack-smashing protector.)

This is roughly the same buffer overflow condition
that was fixed by commit 0ddee9b
This fixes #269.
  • Loading branch information
philhofer authored and williamh committed Dec 24, 2018
1 parent 97e74f9 commit 084877e
Showing 1 changed file with 20 additions and 14 deletions.
34 changes: 20 additions & 14 deletions src/librc/librc-daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,34 +48,40 @@ pid_is_exec(pid_t pid, const char *exec)
static bool
pid_is_argv(pid_t pid, const char *const *argv)
{
char *buffer = NULL;
char *cmdline = NULL;
int fd;
char buffer[PATH_MAX];
char *p;
ssize_t bytes;
size_t bytes;
bool rc;

xasprintf(&cmdline, "/proc/%u/cmdline", pid);
if ((fd = open(cmdline, O_RDONLY)) < 0) {
if (!rc_getfile(cmdline, &buffer, &bytes)) {
free(cmdline);
return false;
}
bytes = read(fd, buffer, sizeof(buffer));
close(fd);
free(cmdline);
if (bytes == -1)
if (bytes <= 0) {
if (buffer)
free(buffer);
return false;

buffer[bytes] = '\0';
}
p = buffer;
rc = true;
while (*argv) {
if (strcmp(*argv, p) != 0)
return false;
if (strcmp(*argv, p) != 0) {
rc = false;
break;
}

argv++;
p += strlen(p) + 1;
if ((unsigned)(p - buffer) > sizeof(buffer))
return false;
if ((unsigned)(p - buffer) >= bytes) {
rc = false;
break;
}
}
return true;
free(buffer);
return rc;
}

RC_PIDLIST *
Expand Down

0 comments on commit 084877e

Please sign in to comment.