[vlc-devel] [PATCH] Possible buffer overflow in vlc_readdir (fixes #9676)

[Iwao AVE!]

In vlc_readdir(), the buffer for dirent is too small for a filename with multibyte characters (e.g. utf-8, utf-16, etc.) and causes buffer overflow as explained in the comment.

This patch replaces readdir_r() with readdir(), but it should be still thread-safe because the directory stream is not shared (see the articles below).
If using readdir() is not an option, it might be possible to workaround the bug by allocating four bytes for each character.

http://elliotth.blogspot.jp/2012/10/how-not-to-use-readdirr3.html
http://womble.decadent.org.uk/readdir_r-advisory.html
http://udrepper.livejournal.com/18555.html
---
 src/posix/filesystem.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/src/posix/filesystem.c b/src/posix/filesystem.c
index b008a6c..2a0496b 100644
--- a/src/posix/filesystem.c
+++ b/src/posix/filesystem.c
@@ -150,28 +150,14 @@ DIR *vlc_opendir (const char *dirname)
  */
 char *vlc_readdir( DIR *dir )
 {
-    /* Beware that readdir_r() assumes <buf> is large enough to hold the result
-     * dirent including the file name. A buffer overflow could occur otherwise.
-     * In particular, pathconf() and _POSIX_NAME_MAX cannot be used here. */
     struct dirent *ent;
     char *path = NULL;
 
-    long len = fpathconf (dirfd (dir), _PC_NAME_MAX);
-    /* POSIX says there shall be room for NAME_MAX bytes at all times */
-    if (len == -1 || len < NAME_MAX)
-        len = NAME_MAX;
-    len += sizeof (*ent) + 1 - sizeof (ent->d_name);
-
-    struct dirent *buf = malloc (len);
-    if (unlikely(buf == NULL))
+    ent = readdir(dir);
+    if (ent == NULL)
         return NULL;
-
-    int val = readdir_r (dir, buf, &ent);
-    if (val != 0)
-        errno = val;
-    else if (ent != NULL)
+    else
         path = strdup (ent->d_name);
-    free (buf);
     return path;
 }
 
-- 
1.8.5.2