From 18c89b4b9869ddf4eb0796c0f5e58847958235fc Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Mon, 5 May 2014 18:12:25 +0930 Subject: [PATCH] Fix bug in read_symlink(), improve logging --- os.c | 8 ++++---- os.h | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/os.c b/os.c index 7cfc6877..1e6b3817 100644 --- a/os.c +++ b/os.c @@ -171,14 +171,14 @@ ssize_t read_symlink(const char *path, char *buf, size_t len) if (len == 0) { struct stat stat; if (lstat(path, &stat) == -1) - return WHYF_perror("lstat(%s)", path); - return stat.st_size; + return WHYF_perror("lstat(%s)", alloca_str_toprint(path)); + return stat.st_size + 1; // allow for terminating nul } ssize_t nr = readlink(path, buf, len); if (nr == -1) - return WHYF_perror("readlink(%s,%p,%zu)", path, buf, len); + return WHYF_perror("readlink(%s,%p,%zu)", alloca_str_toprint(path), buf, len); if ((size_t)nr >= len) - return WHYF("buffer overrun from readlink(%s, len=%zu)", path, len); + return WHYF("buffer overrun from readlink(%s, len=%zu)", alloca_str_toprint(path), len); buf[nr] = '\0'; return nr; } diff --git a/os.h b/os.h index a857ebf6..4154e41e 100644 --- a/os.h +++ b/os.h @@ -133,12 +133,12 @@ int urandombytes(unsigned char *buf, size_t len); * content and the terminating nul. If readlink(2) returns an error, then logs * an ERROR and returns -1. Otherwise, returns the number of bytes read, * including the terminating nul, ie, returns what readlink(2) returns plus - * one. If the 'len' argument is given as zero, then returns the number of - * bytes that would be read, by calling lstat(2) instead of readlink(2), plus - * one for the terminating nul. Beware of the following race condition: a - * symbolic link may be altered between calling the lstat(2) and readlink(2), - * so the following apparently overflow-proof code may still fail from a buffer - * overflow in the second call to read_symlink(): + * one. If the 'len' argument is given as zero, then ignores 'buf' and returns + * the number of bytes that would be read, by calling lstat(2) instead of + * readlink(2), plus one for the terminating nul. Beware of the following race + * condition: a symbolic link may be altered between calling the lstat(2) and + * readlink(2), so the following apparently overflow-proof code may still fail + * from a buffer overflow in the second call to read_symlink(): * * char *readlink_malloc(const char *path) { * ssize_t len = read_symlink(path, NULL, 0);