Fix one possible buffer overflow and one underflow. Also some minor

cleanups.  From Jan Kokemueller.  OK deraadt@

diff --git a/lib/libc/stdlib/realpath.c b/lib/libc/stdlib/realpath.c
index c691252..5d226d9 100644 (file)
--- a/lib/libc/stdlib/realpath.c
+++ b/lib/libc/stdlib/realpath.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: realpath.c,v 1.21 2016/08/28 04:08:59 guenther Exp $ */
+/*     $OpenBSD: realpath.c,v 1.22 2017/12/24 01:50:50 millert Exp $ */
 /*
  * Copyright (c) 2003 Constantin S. Svintsoff <kostik@iclub.nsu.ru>
  *
@@ -45,12 +45,19 @@
 char *
 realpath(const char *path, char *resolved)
 {
-       char *p, *q, *s;
-       size_t left_len, resolved_len;
+       const char *p;
+       char *q;
+       size_t left_len, resolved_len, next_token_len;
        unsigned symlinks;
-       int serrno, slen, mem_allocated;
+       int serrno, mem_allocated;
+       ssize_t slen;
        char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
 
+       if (path == NULL) {
+               errno = EINVAL;
+               return (NULL);
+       }
+
        if (path[0] == '\0') {
                errno = ENOENT;
                return (NULL);
@@ -85,7 +92,7 @@ realpath(const char *path, char *resolved)
                resolved_len = strlen(resolved);
                left_len = strlcpy(left, path, sizeof(left));
        }
-       if (left_len >= sizeof(left) || resolved_len >= PATH_MAX) {
+       if (left_len >= sizeof(left)) {
                errno = ENAMETOOLONG;
                goto err;
        }
@@ -99,16 +106,19 @@ realpath(const char *path, char *resolved)
                 * and its length.
                 */
                p = strchr(left, '/');
-               s = p ? p : left + left_len;
-               if (s - left >= sizeof(next_token)) {
-                       errno = ENAMETOOLONG;
-                       goto err;
+
+               next_token_len = p ? (size_t) (p - left) : left_len;
+               memcpy(next_token, left, next_token_len);
+               next_token[next_token_len] = '\0';
+
+               if (p != NULL) {
+                       left_len -= next_token_len + 1;
+                       memmove(left, p + 1, left_len + 1);
+               } else {
+                       left[0] = '\0';
+                       left_len = 0;
                }
-               memcpy(next_token, left, s - left);
-               next_token[s - left] = '\0';
-               left_len -= s - left;
-               if (p != NULL)
-                       memmove(left, s + 1, left_len + 1);
+
                if (resolved[resolved_len - 1] != '/') {
                        if (resolved_len + 1 >= PATH_MAX) {
                                errno = ENAMETOOLONG;
@@ -136,16 +146,17 @@ realpath(const char *path, char *resolved)
                }
 
                /*
-                * Append the next path component and lstat() it. If
-                * lstat() fails we still can return successfully if
-                * there are no more path components left.
+                * Append the next path component and readlink() it. If
+                * readlink() fails we still can return successfully if
+                * it exists but isn't a symlink, or if there are no more
+                * path components left.
                 */
                resolved_len = strlcat(resolved, next_token, PATH_MAX);
                if (resolved_len >= PATH_MAX) {
                        errno = ENAMETOOLONG;
                        goto err;
                }
-               slen = readlink(resolved, symlink, sizeof(symlink) - 1);
+               slen = readlink(resolved, symlink, sizeof(symlink));
                if (slen < 0) {
                        switch (errno) {
                        case EINVAL:
@@ -160,6 +171,12 @@ realpath(const char *path, char *resolved)
                        default:
                                goto err;
                        }
+               } else if (slen == 0) {
+                       errno = EINVAL;
+                       goto err;
+               } else if (slen == sizeof(symlink)) {
+                       errno = ENAMETOOLONG;
+                       goto err;
                } else {
                        if (symlinks++ > SYMLOOP_MAX) {
                                errno = ELOOP;
@@ -170,9 +187,8 @@ realpath(const char *path, char *resolved)
                        if (symlink[0] == '/') {
                                resolved[1] = 0;
                                resolved_len = 1;
-                       } else if (resolved_len > 1) {
+                       } else {
                                /* Strip the last path component. */
-                               resolved[resolved_len - 1] = '\0';
                                q = strrchr(resolved, '/') + 1;
                                *q = '\0';
                                resolved_len = q - resolved;

diff --git a/libexec/ld.so/dl_realpath.c b/libexec/ld.so/dl_realpath.c
index a11389a..ac80530 100644 (file)
--- a/libexec/ld.so/dl_realpath.c
+++ b/libexec/ld.so/dl_realpath.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: dl_realpath.c,v 1.5 2017/08/27 21:59:49 deraadt Exp $ */
+/*     $OpenBSD: dl_realpath.c,v 1.6 2017/12/24 01:50:50 millert Exp $ */
 /*
  * Copyright (c) 2003 Constantin S. Svintsoff <kostik@iclub.nsu.ru>
  *
@@ -47,13 +47,18 @@
 char *
 _dl_realpath(const char *path, char *resolved)
 {
-       const char *p, *s;
+       const char *p;
        char *q;
-       size_t left_len, resolved_len;
+       size_t left_len, resolved_len, next_token_len;
        unsigned symlinks;
-       int slen, mem_allocated;
+       int mem_allocated;
+       ssize_t slen;
        char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
 
+       if (path == NULL) {
+               return (NULL);
+       }
+
        if (path[0] == '\0') {
                return (NULL);
        }
@@ -85,7 +90,7 @@ _dl_realpath(const char *path, char *resolved)
                resolved_len = _dl_strlen(resolved);
                left_len = _dl_strlcpy(left, path, sizeof(left));
        }
-       if (left_len >= sizeof(left) || resolved_len >= PATH_MAX) {
+       if (left_len >= sizeof(left)) {
                goto err;
        }
 
@@ -98,15 +103,19 @@ _dl_realpath(const char *path, char *resolved)
                 * and its length.
                 */
                p = _dl_strchr(left, '/');
-               s = p ? p : left + left_len;
-               if (s - left >= sizeof(next_token)) {
-                       goto err;
+
+               next_token_len = p ? (size_t) (p - left) : left_len;
+               _dl_bcopy(left, next_token, next_token_len);
+               next_token[next_token_len] = '\0';
+
+               if (p != NULL) {
+                       left_len -= next_token_len + 1;
+                       _dl_bcopy(p + 1, left, left_len + 1);
+               } else {
+                       left[0] = '\0';
+                       left_len = 0;
                }
-               _dl_bcopy(left, next_token, s - left);
-               next_token[s - left] = '\0';
-               left_len -= s - left;
-               if (p != NULL)
-                       _dl_bcopy(s + 1, left, left_len + 1);
+
                if (resolved[resolved_len - 1] != '/') {
                        if (resolved_len + 1 >= PATH_MAX) {
                                goto err;
@@ -135,14 +144,14 @@ _dl_realpath(const char *path, char *resolved)
                /*
                 * Append the next path component and readlink() it. If
                 * readlink() fails we still can return successfully if
-                * it wasn't a exists but isn't a symlink, or if there
-                * are no more path components left.
+                * it exists but isn't a symlink, or if there are no more
+                * path components left.
                 */
                resolved_len = _dl_strlcat(resolved, next_token, PATH_MAX);
                if (resolved_len >= PATH_MAX) {
                        goto err;
                }
-               slen = _dl_readlink(resolved, symlink, sizeof(symlink) - 1);
+               slen = _dl_readlink(resolved, symlink, sizeof(symlink));
                if (slen < 0) {
                        switch (slen) {
                        case -EINVAL:
@@ -155,6 +164,10 @@ _dl_realpath(const char *path, char *resolved)
                        default:
                                goto err;
                        }
+               } else if (slen == 0) {
+                       goto err;
+               } else if (slen == sizeof(symlink)) {
+                       goto err;
                } else {
                        if (symlinks++ > SYMLOOP_MAX) {
                                goto err;
@@ -166,7 +179,6 @@ _dl_realpath(const char *path, char *resolved)
                                resolved_len = 1;
                        } else if (resolved_len > 1) {
                                /* Strip the last path component. */
-                               resolved[resolved_len - 1] = '\0';
                                q = _dl_strrchr(resolved, '/') + 1;
                                *q = '\0';
                                resolved_len = q - resolved;