[PATCH] Read past buffer end in readdir_r

[Federico Terraneo <fede dot tft at hotmail dot it>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to POSIX "The array [...] d_name in struct dirent is not
fixed size", but in libc/posix/readdir_r.c there's a

tmpdp = (struct dirent *)(dirp->dd_buf + dirp->dd_loc);
memcpy (dp, tmpdp, sizeof(struct dirent));

Note that the memcpy reads from dirp->dd_buf + dirp->dd_loc, and
writes into a struct dirent, with a fixed size of sizeof(struct dirent).
The target of the memcpy is safe, as d_name in struct dirent is defined as

char d_name[NAME_MAX + 1];

or the maximum possible size, but the source buffer is not, as the OS
may pack many struct dirent relying on d_reclen to jump from one to
the next one.

This looks like it may cause a read past dirp->dd_buf. Considering that
NAME_MAX is usually 255, and the minimum file name is one char, the
memcpy may read up to 254 bytes past dirp->dd_buf.

Going into opendir, dd_buf is heap allocated, and if by chance it's
allocated just under the program's brk, reading past it  may well
cause a segfault.

I'd like to know what you think about this, attached you'll find a patch
to fix the issue.

newlib/ChangeLog
2013-06-13  Terraneo Federico  <fede.tft@hotmail.it>

	* libc/posix/readdir_r.c: Fix potential read past dirp->dd_buf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRvh9iAAoJECkLFtN5Xr9f3pQIAIqT6OPmAADwvsuZtBQQZzxs
Jhx4FjNsdevBDHGDRWgJKvw9GZ01h4WzHXUj72le7gNYXdL3BCR9ty9mYcceiPDa
V3Wd/+bmPCEUKPDi4oksDBVnKjPmS6Nt7PkmW2Xg/4rTwC3fghjTtQv62H6moJHM
v5vlUP7LDimtfjwzvr18vjrCip8WIvP3vTI+v3Fz6fuhb8iwwJlbUh1QGuBJMGa5
hcFH2hMySJ+Kioob+mmKrrLvg9/4xQXbCadzcA88UDbgcsOySe43NCJoURS97Avp
fWp8LlUetco6A2k7k3TvK7ETU9fkeUb6HkNEsBFF2ae0hleB+OugLJc7J3zfqYE=
=RG3m
-----END PGP SIGNATURE-----

diff -ruN a/newlib/libc/posix/readdir_r.c b/newlib/libc/posix/readdir_r.c
--- a/newlib/libc/posix/readdir_r.c	2012-03-02 17:02:04.000000000 +0100
+++ b/newlib/libc/posix/readdir_r.c	2013-06-13 10:52:57.511346711 +0200
@@ -43,6 +43,10 @@
 #include <errno.h>
 #include <string.h>
 
+#ifndef MIN
+#define MIN(a,b) ((a) < (b) ? (a) : (b))
+#endif
+
 extern int getdents (int fd, void *dp, int count);
 
 /*
@@ -84,16 +88,17 @@
       continue;
     }
     tmpdp = (struct dirent *)(dirp->dd_buf + dirp->dd_loc);
-    memcpy (dp, tmpdp, sizeof(struct dirent));
 
-    if (dp->d_reclen <= 0 ||
-	dp->d_reclen > dirp->dd_len + 1 - dirp->dd_loc) {
+    if (tmpdp->d_reclen <= 0 ||
+	tmpdp->d_reclen > dirp->dd_len + 1 - dirp->dd_loc) {
 #ifdef HAVE_DD_LOCK
       __lock_release_recursive(dirp->dd_lock);
 #endif
       *dpp = NULL;
       return -1;
     }
+    memcpy (dp, tmpdp, MIN(tmpdp->d_reclen,sizeof(struct dirent)));
+    
     dirp->dd_loc += dp->d_reclen;
     if (dp->d_ino == 0)
       continue;

Attachment: readdir_r.patch.sig
Description: Binary data