PATCH: Stack-based buffer overflow in gen_matches_files() at compctl.c

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

compctl.c:gen_matches_files() declares a local PATH_MAX-sized buffer
which is used for storing the completion prefix along with completion
candidate directories. It doesn't check that the current path actually
fits into the buffer, however. This bug corresponds to CVE-2018-1083 and
was reported off-list.

The patch adds a check in three places. This is tricky to test. I've
managed to check all three branches - it now drops the completion
candidate from consideration. But it would be good to have some more
eyes on this. In particular, might any of the tests be off-by-one?

Note that this issue only affects code that the vast majority of users
wouldn't exercise. In a minimally configured zsh setup (using compinit)
filename completion is handled by different code.

Oliver

diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index e9d165780..87d13afc1 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -2176,6 +2176,8 @@ gen_matches_files(int dirs, int execs, int all)
     if (prpre && *prpre) {
 	pathpref = dupstring(prpre);
 	unmetafy(pathpref, &pathpreflen);
+	if (pathpreflen > PATH_MAX)
+	    return;
 	/* system needs NULL termination, not provided by unmetafy */
 	pathpref[pathpreflen] = '\0';
     } else {
@@ -2218,6 +2220,8 @@ gen_matches_files(int dirs, int execs, int all)
 		     * the path buffer by appending the filename.       */
 		    ums = dupstring(n);
 		    unmetafy(ums, &umlen);
+		    if (umlen + pathpreflen + 1 > PATH_MAX)
+			continue;
 		    memcpy(q, ums, umlen);
 		    q[umlen] = '\0';
 		    /* And do the stat. */
@@ -2232,6 +2236,8 @@ gen_matches_files(int dirs, int execs, int all)
 			/* We have to test for a path suffix. */
 			int o = strlen(p), tt;
 
+			if (o + strlen(psuf) > PATH_MAX)
+			    continue;
 			/* Append it to the path buffer. */
 			strcpy(p + o, psuf);