On Fri, 22 Dec 2017 12:08:58 +0100, =?UTF-8?Q?Jan_Kokem=c3=bcller?= wrote: > I've found some buffer overflows in realpath(3). They are limited to > just two bytes though (one after the 'left' buffer and one before > 'symlink'), so the impact is minimal. > > Similar bugs in FreeBSD: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219154 > > > Here is a list of issues: > > - The statement "left_len -= s - left;" does not take the slash into > account if one was found. This results in the invariant > "left[left_len] == '\0'" being violated (and possible buffer > overflows). The patch replaces the variable "s" with a size_t > "next_token_len" for more clarity. > > - "slen" from readlink(2) can be 0 when encountering empty symlinks. > Then, further down, "symlink[slen - 1]" underflows the buffer. When > slen == 0, realpath(3) should probably return ENOENT > (http://austingroupbugs.net/view.php?id=825, > https://lwn.net/Articles/551224/). > > - return ENOENT if path is NULL (POSIX requires this) > > - The condition "resolved_len >= PATH_MAX" cannot be true. > > - Similarly, "s - left >= sizeof(next_token)" cannot be true, as long > as "sizeof(next_token) >= sizeof(left)". > > - Return ENAMETOOLONG when a resolved symlink from readlink(2) is too > long for the symlink buffer (instead of just truncating it). > > - "resolved_len > 1" below the call to readlink(2) is always true as > "strlcat(resolved, next_token, PATH_MAX);" always results in a string > of length > 1. Also, "resolved[resolved_len - 1] = '\0';" is not > needed; there can never be a trailing slash here. > > > Here is a patch. I must admit that I haven't tested this on OpenBSD but > compiled the OpenBSD version on FreeBSD. I have left in some asserts to > describe loop invariants. Those can be removed I guess. I've just > noticed the realpath copy in 'libexec/ld.so/dl_realpath.c' that should > be fixed similarly.
The patch looks correct to me. We can either remove the asserts() or simply define NDEBUG before including assert.h. Anyone else want to OK this? - todd
