]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
CVE-2013-4237, BZ #14699: Buffer overflow in readdir_r
authorFlorian Weimer <fweimer@redhat.com>
Fri, 16 Aug 2013 07:38:52 +0000 (09:38 +0200)
committerAdhemerval Zanella <azanella@linux.vnet.ibm.com>
Thu, 15 Jan 2015 19:56:56 +0000 (14:56 -0500)
* sysdeps/posix/dirstream.h (struct __dirstream): Add errcode
member.
* sysdeps/posix/opendir.c (__alloc_dir): Initialize errcode
member.
* sysdeps/posix/rewinddir.c (rewinddir): Reset errcode member.
* sysdeps/posix/readdir_r.c (__READDIR_R): Enforce NAME_MAX limit.
Return delayed error code.  Remove GETDENTS_64BIT_ALIGNED
conditional.
* sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c: Do not define
GETDENTS_64BIT_ALIGNED.
* sysdeps/unix/sysv/linux/i386/readdir64_r.c: Likewise.
* manual/filesys.texi (Reading/Closing Directory): Document
ENAMETOOLONG return value of readdir_r.  Recommend readdir more
strongly.
* manual/conf.texi (Limits for Files): Add portability note to
NAME_MAX, PATH_MAX.
(Pathconf): Add portability note for _PC_NAME_MAX, _PC_PATH_MAX.

ChangeLog
NEWS
manual/conf.texi
manual/filesys.texi
sysdeps/unix/dirstream.h
sysdeps/unix/opendir.c
sysdeps/unix/readdir_r.c
sysdeps/unix/rewinddir.c
sysdeps/unix/sysv/linux/i386/readdir64_r.c
sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c

index 05c13c03e4c1b87a086093bda292cf6a18704ed1..9997c1cae53b1b8dd3ee5cd46e1b024c731e812c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+2013-08-16  Florian Weimer  <fweimer@redhat.com>
+
+       [BZ #14699]
+       CVE-2013-4237
+       * sysdeps/posix/dirstream.h (struct __dirstream): Add errcode
+       member.
+       * sysdeps/posix/opendir.c (__alloc_dir): Initialize errcode
+       member.
+       * sysdeps/posix/rewinddir.c (rewinddir): Reset errcode member.
+       * sysdeps/posix/readdir_r.c (__READDIR_R): Enforce NAME_MAX limit.
+       Return delayed error code.  Remove GETDENTS_64BIT_ALIGNED
+       conditional.
+       * sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c: Do not define
+       GETDENTS_64BIT_ALIGNED.
+       * sysdeps/unix/sysv/linux/i386/readdir64_r.c: Likewise.
+       * manual/filesys.texi (Reading/Closing Directory): Document
+       ENAMETOOLONG return value of readdir_r.  Recommend readdir more
+       strongly.
+       * manual/conf.texi (Limits for Files): Add portability note to
+       NAME_MAX, PATH_MAX.
+       (Pathconf): Add portability note for _PC_NAME_MAX, _PC_PATH_MAX.
+
 2013-09-23  Carlos O'Donell  <carlos@redhat.com>
 
        [BZ #15754]
diff --git a/NEWS b/NEWS
index ceebbbb43ead5d13ab49da529dac82273258de75..b2543182d71c915d7fac5c4796087601ff6f15ac 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,8 +9,12 @@ Version 2.16.1
 
 * The following bugs are resolved with this release:
 
-  6530, 14195, 14547, 14459, 14476, 14562, 14621, 14648, 14756, 14831, 15078,
-  15754, 15755, 16072.
+  6530, 14195, 14547, 14459, 14476, 14562, 14621, 14648, 14699, 14756, 14831,
+  15078, 15754, 15755, 16072.
+
+* CVE-2013-4237 The readdir_r function could write more than NAME_MAX bytes
+  to the d_name member of struct dirent, or omit the terminating NUL
+  character.  (Bugzilla #14699).
 
 * CVE-2013-4788 The pointer guard used for pointer mangling was not
   initialized for static applications resulting in the security feature
index 61dc260317f560263a432982f6c6f06a2afbe03f..f2bd33a667b981bdc08323d058ed730b0139552d 100644 (file)
@@ -1474,6 +1474,9 @@ Inquire about the value of @code{POSIX_REC_MIN_XFER_SIZE}.
 Inquire about the value of @code{POSIX_REC_XFER_ALIGN}.
 @end table
 
+@strong{Portability Note:} On some systems, @theglibc{} does not
+enforce @code{_PC_NAME_MAX} or @code{_PC_PATH_MAX} limits.
+
 @node Utility Limits
 @section Utility Program Capacity Limits
 
index 7003f9c473aaddc5075c072c204e4b009db390c6..e2784688f8e99ec8e87e6666ed9a0cf1484dbb02 100644 (file)
@@ -444,9 +444,9 @@ symbols are declared in the header file @file{dirent.h}.
 @comment POSIX.1
 @deftypefun {struct dirent *} readdir (DIR *@var{dirstream})
 This function reads the next entry from the directory.  It normally
-returns a pointer to a structure containing information about the file.
-This structure is statically allocated and can be rewritten by a
-subsequent call.
+returns a pointer to a structure containing information about the
+file.  This structure is associated with the @var{dirstream} handle
+and can be rewritten by a subsequent call.
 
 @strong{Portability Note:} On some systems @code{readdir} may not
 return entries for @file{.} and @file{..}, even though these are always
@@ -461,19 +461,61 @@ conditions are defined for this function:
 The @var{dirstream} argument is not valid.
 @end table
 
-@code{readdir} is not thread safe.  Multiple threads using
-@code{readdir} on the same @var{dirstream} may overwrite the return
-value.  Use @code{readdir_r} when this is critical.
+To distinguish between an end-of-directory condition or an error, you
+must set @code{errno} to zero before calling @code{readdir}.  To avoid
+entering an infinite loop, you should stop reading from the directory
+after the first error.
+
+In POSIX.1-2008, @code{readdir} is not thread-safe.  In @theglibc{}
+implementation, it is safe to call @code{readdir} concurrently on
+different @var{dirstream}s, but multiple threads accessing the same
+@var{dirstream} result in undefined behavior.  @code{readdir_r} is a
+fully thread-safe alternative, but suffers from poor portability (see
+below).  It is recommended that you use @code{readdir}, with external
+locking if multiple threads access the same @var{dirstream}.
 @end deftypefun
 
 @comment dirent.h
 @comment GNU
 @deftypefun int readdir_r (DIR *@var{dirstream}, struct dirent *@var{entry}, struct dirent **@var{result})
-This function is the reentrant version of @code{readdir}.  Like
-@code{readdir} it returns the next entry from the directory.  But to
-prevent conflicts between simultaneously running threads the result is
-not stored in statically allocated memory.  Instead the argument
-@var{entry} points to a place to store the result.
+This function is a version of @code{readdir} which performs internal
+locking.  Like @code{readdir} it returns the next entry from the
+directory.  To prevent conflicts between simultaneously running
+threads the result is stored inside the @var{entry} object.
+
+@strong{Portability Note:} It is recommended to use @code{readdir}
+instead of @code{readdir_r} for the following reasons:
+
+@itemize @bullet
+@item
+On systems which do not define @code{NAME_MAX}, it may not be possible
+to use @code{readdir_r} safely because the caller does not specify the
+length of the buffer for the directory entry.
+
+@item
+On some systems, @code{readdir_r} cannot read directory entries with
+very long names.  If such a name is encountered, @theglibc{}
+implementation of @code{readdir_r} returns with an error code of
+@code{ENAMETOOLONG} after the final directory entry has been read.  On
+other systems, @code{readdir_r} may return successfully, but the
+@code{d_name} member may not be NUL-terminated or may be truncated.
+
+@item
+POSIX-1.2008 does not guarantee that @code{readdir} is thread-safe,
+even when access to the same @var{dirstream} is serialized.  But in
+current implementations (including @theglibc{}), it is safe to call
+@code{readdir} concurrently on different @var{dirstream}s, so there is
+no need to use @code{readdir_r} in most multi-threaded programs.  In
+the rare case that multiple threads need to read from the same
+@var{dirstream}, it is still better to use @code{readdir} and external
+synchronization.
+
+@item
+It is expected that future versions of POSIX will obsolete
+@code{readdir_r} and mandate the level of thread safety for
+@code{readdir} which is provided by @theglibc{} and other
+implementations today.
+@end itemize
 
 Normally @code{readdir_r} returns zero and sets @code{*@var{result}}
 to @var{entry}.  If there are no more entries in the directory or an
@@ -481,15 +523,6 @@ error is detected, @code{readdir_r} sets @code{*@var{result}} to a
 null pointer and returns a nonzero error code, also stored in
 @code{errno}, as described for @code{readdir}.
 
-@strong{Portability Note:} On some systems @code{readdir_r} may not
-return a NUL terminated string for the file name, even when there is no
-@code{d_reclen} field in @code{struct dirent} and the file
-name is the maximum allowed size.  Modern systems all have the
-@code{d_reclen} field, and on old systems multi-threading is not
-critical.  In any case there is no such problem with the @code{readdir}
-function, so that even on systems without the @code{d_reclen} member one
-could use multiple threads by using external locking.
-
 It is also important to look at the definition of the @code{struct
 dirent} type.  Simply passing a pointer to an object of this type for
 the second parameter of @code{readdir_r} might not be enough.  Some
index 6ca290471d56120f09bf4b5eb86b84d9b353e221..e31cdf32bd2900d9ebf440bf127507ab3fe3c9cf 100644 (file)
@@ -39,6 +39,8 @@ struct __dirstream
 
     off_t filepos;             /* Position of next entry to read.  */
 
+    int errcode;               /* Delayed error code.  */
+
     /* Directory block.  */
     char data[0] __attribute__ ((aligned (__alignof__ (void*))));
   };
index e093142f62654575ba79085209f9d1cbaf150dd0..916d7bca734eaf264201a642e44ecf99826a6860 100644 (file)
@@ -223,6 +223,7 @@ __alloc_dir (int fd, bool close_fd, int flags, const struct stat64 *statp)
   dirp->size = 0;
   dirp->offset = 0;
   dirp->filepos = 0;
+  dirp->errcode = 0;
 
   return dirp;
 }
index bfa2c0b8f516b386340df8c08a27a45d312a2061..878693bbcd66d2441a12246f97e2a5101587b5a4 100644 (file)
@@ -41,6 +41,7 @@ __READDIR_R (DIR *dirp, DIRENT_TYPE *entry, DIRENT_TYPE **result)
   DIRENT_TYPE *dp;
   size_t reclen;
   const int saved_errno = errno;
+  int ret;
 
   __libc_lock_lock (dirp->lock);
 
@@ -71,10 +72,10 @@ __READDIR_R (DIR *dirp, DIRENT_TYPE *entry, DIRENT_TYPE **result)
                  bytes = 0;
                  __set_errno (saved_errno);
                }
+             if (bytes < 0)
+               dirp->errcode = errno;
 
              dp = NULL;
-             /* Reclen != 0 signals that an error occurred.  */
-             reclen = bytes != 0;
              break;
            }
          dirp->size = (size_t) bytes;
@@ -107,29 +108,46 @@ __READDIR_R (DIR *dirp, DIRENT_TYPE *entry, DIRENT_TYPE **result)
       dirp->filepos += reclen;
 #endif
 
-      /* Skip deleted files.  */
+#ifdef NAME_MAX
+      if (reclen > offsetof (DIRENT_TYPE, d_name) + NAME_MAX + 1)
+       {
+         /* The record is very long.  It could still fit into the
+            caller-supplied buffer if we can skip padding at the
+            end.  */
+         size_t namelen = _D_EXACT_NAMLEN (dp);
+         if (namelen <= NAME_MAX)
+           reclen = offsetof (DIRENT_TYPE, d_name) + namelen + 1;
+         else
+           {
+             /* The name is too long.  Ignore this file.  */
+             dirp->errcode = ENAMETOOLONG;
+             dp->d_ino = 0;
+             continue;
+           }
+       }
+#endif
+
+      /* Skip deleted and ignored files.  */
     }
   while (dp->d_ino == 0);
 
   if (dp != NULL)
     {
-#ifdef GETDENTS_64BIT_ALIGNED
-      /* The d_reclen value might include padding which is not part of
-        the DIRENT_TYPE data structure.  */
-      reclen = MIN (reclen,
-                   offsetof (DIRENT_TYPE, d_name) + sizeof (dp->d_name));
-#endif
       *result = memcpy (entry, dp, reclen);
-#ifdef GETDENTS_64BIT_ALIGNED
+#ifdef _DIRENT_HAVE_D_RECLEN
       entry->d_reclen = reclen;
 #endif
+      ret = 0;
     }
   else
-    *result = NULL;
+    {
+      *result = NULL;
+      ret = dirp->errcode;
+    }
 
   __libc_lock_unlock (dirp->lock);
 
-  return dp != NULL ? 0 : reclen ? errno : 0;
+  return ret;
 }
 
 #ifdef __READDIR_R_ALIAS
index f8eefea542819c35e8f471dec66ebb04ba14a4a3..5e99cb0d75cd3a031d4a6b7a4f4e54ec77ba12c0 100644 (file)
@@ -33,6 +33,7 @@ rewinddir (dirp)
   dirp->filepos = 0;
   dirp->offset = 0;
   dirp->size = 0;
+  dirp->errcode = 0;
 #ifndef NOT_IN_libc
   __libc_lock_unlock (dirp->lock);
 #endif
index 3b3116c01eb2254c166f450a6d74b3fe26bb72d1..bc1f7e546a71e74a16d45e78dd82d9040c672124 100644 (file)
@@ -18,7 +18,6 @@
 #define __READDIR_R __readdir64_r
 #define __GETDENTS __getdents64
 #define DIRENT_TYPE struct dirent64
-#define GETDENTS_64BIT_ALIGNED 1
 
 #include <sysdeps/unix/readdir_r.c>
 
index 12ca1a1ef7412e611e9cc2f9810320718a334cba..adb92db6af9cb90c321f8b9bf1757836d6715141 100644 (file)
@@ -1,5 +1,4 @@
 #define readdir64_r __no_readdir64_r_decl
-#define GETDENTS_64BIT_ALIGNED 1
 #include <sysdeps/unix/readdir_r.c>
 #undef readdir64_r
 weak_alias (__readdir_r, readdir64_r)