303 lines
11 KiB
Diff
303 lines
11 KiB
Diff
|
commit 91ce40854d0b7f865cf5024ef95a8026b76096f3
|
||
|
Author: Florian Weimer <fweimer@redhat.com>
|
||
|
Date: Fri Aug 16 09:38:52 2013 +0200
|
||
|
|
||
|
CVE-2013-4237, BZ #14699: Buffer overflow in readdir_r
|
||
|
|
||
|
* 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.
|
||
|
|
||
|
diff --git a/manual/conf.texi b/manual/conf.texi
|
||
|
index 7eb8b36..c720063 100644
|
||
|
--- a/manual/conf.texi
|
||
|
+++ b/manual/conf.texi
|
||
|
@@ -1149,6 +1149,9 @@ typed ahead as input. @xref{I/O Queues}.
|
||
|
@deftypevr Macro int NAME_MAX
|
||
|
The uniform system limit (if any) for the length of a file name component, not
|
||
|
including the terminating null character.
|
||
|
+
|
||
|
+@strong{Portability Note:} On some systems, @theglibc{} defines
|
||
|
+@code{NAME_MAX}, but does not actually enforce this limit.
|
||
|
@end deftypevr
|
||
|
|
||
|
@comment limits.h
|
||
|
@@ -1157,6 +1160,9 @@ including the terminating null character.
|
||
|
The uniform system limit (if any) for the length of an entire file name (that
|
||
|
is, the argument given to system calls such as @code{open}), including the
|
||
|
terminating null character.
|
||
|
+
|
||
|
+@strong{Portability Note:} @Theglibc{} does not enforce this limit
|
||
|
+even if @code{PATH_MAX} is defined.
|
||
|
@end deftypevr
|
||
|
|
||
|
@cindex limits, pipe buffer size
|
||
|
@@ -1476,6 +1482,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
|
||
|
|
||
|
diff --git a/manual/filesys.texi b/manual/filesys.texi
|
||
|
index 1df9cf2..814c210 100644
|
||
|
--- a/manual/filesys.texi
|
||
|
+++ b/manual/filesys.texi
|
||
|
@@ -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
|
||
|
diff --git a/sysdeps/posix/dirstream.h b/sysdeps/posix/dirstream.h
|
||
|
index a7a074d..8e8570d 100644
|
||
|
--- a/sysdeps/posix/dirstream.h
|
||
|
+++ b/sysdeps/posix/dirstream.h
|
||
|
@@ -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*))));
|
||
|
};
|
||
|
diff --git a/sysdeps/posix/opendir.c b/sysdeps/posix/opendir.c
|
||
|
index ddfc3a7..fc05b0f 100644
|
||
|
--- a/sysdeps/posix/opendir.c
|
||
|
+++ b/sysdeps/posix/opendir.c
|
||
|
@@ -231,6 +231,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;
|
||
|
}
|
||
|
diff --git a/sysdeps/posix/readdir_r.c b/sysdeps/posix/readdir_r.c
|
||
|
index b5a8e2e..8ed5c3f 100644
|
||
|
--- a/sysdeps/posix/readdir_r.c
|
||
|
+++ b/sysdeps/posix/readdir_r.c
|
||
|
@@ -40,6 +40,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);
|
||
|
|
||
|
@@ -70,10 +71,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;
|
||
|
@@ -106,29 +107,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
|
||
|
diff --git a/sysdeps/posix/rewinddir.c b/sysdeps/posix/rewinddir.c
|
||
|
index 2935a8e..d4991ad 100644
|
||
|
--- a/sysdeps/posix/rewinddir.c
|
||
|
+++ b/sysdeps/posix/rewinddir.c
|
||
|
@@ -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
|
||
|
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64_r.c b/sysdeps/unix/sysv/linux/i386/readdir64_r.c
|
||
|
index 8ebbcfd..a7d114e 100644
|
||
|
--- a/sysdeps/unix/sysv/linux/i386/readdir64_r.c
|
||
|
+++ b/sysdeps/unix/sysv/linux/i386/readdir64_r.c
|
||
|
@@ -18,7 +18,6 @@
|
||
|
#define __READDIR_R __readdir64_r
|
||
|
#define __GETDENTS __getdents64
|
||
|
#define DIRENT_TYPE struct dirent64
|
||
|
-#define GETDENTS_64BIT_ALIGNED 1
|
||
|
|
||
|
#include <sysdeps/posix/readdir_r.c>
|
||
|
|
||
|
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c b/sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c
|
||
|
index 5ed8e95..290f2c8 100644
|
||
|
--- a/sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c
|
||
|
+++ b/sysdeps/unix/sysv/linux/wordsize-64/readdir_r.c
|
||
|
@@ -1,5 +1,4 @@
|
||
|
#define readdir64_r __no_readdir64_r_decl
|
||
|
-#define GETDENTS_64BIT_ALIGNED 1
|
||
|
#include <sysdeps/posix/readdir_r.c>
|
||
|
#undef readdir64_r
|
||
|
weak_alias (__readdir_r, readdir64_r)
|