If not using smallbuf and len > sizeof(d_name), Move() is illegal.

Coverity CID 135024: Out-of-bounds access (OVERRUN)

If the len is not <= sizeof(smallbuf), the len is at least
sizeof(smallbuf) + 1, which means at least 257.  Now, if the
sizeof(dirent->d_name) is < 257, the Move() would access bytes
beyond the end of d_name[].  Yes, this would need for the d_namlen
(for example) to be out of sync with d_name[].  But paranoia is good.

Because of the severity of the problem (indicating serious mess),
returning NULL instead of partial result is probably better.

Possible portability problem: can d_name ever be not an array,
but instead a bare char pointer?  If that can happen, the sizeof(d_name)
is wrong, and in that case we have to have some other way of figuring
out the maximum size for a directory entry name.

The smallbuf probably could/should be of size MAXPATHLEN.

1 files changed, 9 insertions, 1 deletions

diff --git a/sv.c b/sv.c
index f2736b7dc9..71c398b872 100644
--- a/sv.c
+++ b/sv.c
@@ -13029,7 +13029,7 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param)
 #if defined(HAS_FCHDIR) && defined(HAS_TELLDIR) && defined(HAS_SEEKDIR)
     DIR *pwd;
     const Direntry_t *dirent;
-    char smallbuf[256];
+    char smallbuf[256]; /* XXX MAXPATHLEN, surely? */
     char *name = NULL;
     STRLEN len = 0;
     long pos;
@@ -13079,6 +13079,14 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param)
     pos = PerlDir_tell(dp);
     if ((dirent = PerlDir_read(dp))) {
 	len = d_namlen(dirent);
+        if (len > sizeof(dirent->d_name)) {
+            /* If the len is somehow magically longer than the
+             * maximum length of the directory entry, even though
+             * we could fit it in a buffer, we could not copy it
+             * from the dirent.  Bail out. */
+            PerlDir_close(ret);
+            return (DIR*)NULL;
+        }
 	if (len <= sizeof smallbuf) name = smallbuf;
 	else Newx(name, len, char);
 	Move(dirent->d_name, name, len, char);