Author: kib
Date: Mon May 15 17:14:53 2017
New Revision: 318298
URL: https://svnweb.freebsd.org/changeset/base/318298

Log:
  Fix several buffer overflows in realpath(3).
  - 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/).
  
  Some other minor issues:
  - 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.
  - The truncation check for "strlcat(symlink, left, sizeof(symlink));"
    should be against "sizeof(symlink)" (the third argument to strlcat)
    instead of "sizeof(left)".
  
  Submitted by: Jan Kokemц╪ller <jan.kokemuel...@gmail.com>
  PR:   219154
  MFC after:    2 weeks

Modified:
  head/lib/libc/stdlib/realpath.c

Modified: head/lib/libc/stdlib/realpath.c
==============================================================================
--- head/lib/libc/stdlib/realpath.c     Mon May 15 16:53:02 2017        
(r318297)
+++ head/lib/libc/stdlib/realpath.c     Mon May 15 17:14:53 2017        
(r318298)
@@ -51,10 +51,11 @@ char *
 realpath(const char * __restrict path, char * __restrict resolved)
 {
        struct stat sb;
-       char *p, *q, *s;
-       size_t left_len, resolved_len;
+       char *p, *q;
+       size_t left_len, resolved_len, next_token_len;
        unsigned symlinks;
-       int m, slen;
+       int m;
+       ssize_t slen;
        char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
 
        if (path == NULL) {
@@ -109,18 +110,19 @@ realpath(const char * __restrict path, c
                 * and its length.
                 */
                p = strchr(left, '/');
-               s = p ? p : left + left_len;
-               if (s - left >= sizeof(next_token)) {
-                       if (m)
-                               free(resolved);
-                       errno = ENAMETOOLONG;
-                       return (NULL);
+
+               next_token_len = p ? 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) {
                                if (m)
@@ -173,19 +175,25 @@ realpath(const char * __restrict path, c
                                errno = ELOOP;
                                return (NULL);
                        }
-                       slen = readlink(resolved, symlink, sizeof(symlink) - 1);
-                       if (slen < 0) {
+                       slen = readlink(resolved, symlink, sizeof(symlink));
+                       if (slen <= 0 || slen >= sizeof(symlink)) {
                                if (m)
                                        free(resolved);
+                               if (slen < 0) {
+                                       /* keep errno from readlink(2) call */
+                               } else if (slen == 0) {
+                                       errno = ENOENT;
+                               } else {
+                                       errno = ENAMETOOLONG;
+                               }
                                return (NULL);
                        }
                        symlink[slen] = '\0';
                        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;
@@ -209,7 +217,7 @@ realpath(const char * __restrict path, c
                                }
                                left_len = strlcat(symlink, left,
                                    sizeof(symlink));
-                               if (left_len >= sizeof(left)) {
+                               if (left_len >= sizeof(symlink)) {
                                        if (m)
                                                free(resolved);
                                        errno = ENAMETOOLONG;
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to