Re: [Bug] Strange Globing Behaviour when used with sudo

["Jun T." <takimoto-j@xxxxxxxxxxxxxxxxx>]

Do we need to fix (or workaround) this problem, given that
very basic commands like "ls" are also broken on macOS?

> 2018/06/03 03:17, Daniel Tameling <tamelingdaniel@xxxxxxxxx> wrote:
> 
> $ sudo zsh -c 'echo */../
> file/../

Thanks.

If we really need to workaround this bug, probably the simplest way 
would be not to add a file to pathbuf as in the patch below.

I couldn't put all the patch within a single "#ifdef __APPLE__" because
the return value of addpath() is changed to int.

> Btw:
> I noticed bash manages to trigger the ls error message when zsh
> outputs "no matches found" 
> $ bash -c 'ls */..'
> ls: cannot access '*/..': No such file or directory

Try 'setopt no_nomatch'. See the option NOMATCH in zshoptions(1).


diff --git a/Src/glob.c b/Src/glob.c
index 66a95329f..a36d76ac1 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -259,11 +259,28 @@ struct complist {
 /* Add a component to pathbuf: This keeps track of how    *
  * far we are into a file name, since each path component *
  * must be matched separately.                            */
+#ifdef __APPLE__
+#define BROKEN_STAT
+/*
+ * Workaround for broken stat()/lstat()/access() on macOS.
+ * If called by root, these syscalls mistakenly returns success for paths
+ * like "foo/bar/.", "foo/bar/..", "foo/bar/../" etc. even if bar is a file.
+ * This causes statfullpath() to also succeed for these paths.
+ * To workaround this problem, we do not add s to pathbuf if it is not a
+ * directory and return -1 instead.
+ *
+ * On systems other than macOS, always add s to pathbuf and return 0.
+ */
+#endif
 
 /**/
-static void
+static int
 addpath(char *s, int l)
 {
+#ifdef BROKEN_STAT
+    struct stat stbuf;
+    int oppos = pathpos;
+#endif
     DPUTS(!pathbuf, "BUG: pathbuf not initialised");
     while (pathpos + l + 1 >= pathbufsz)
 	pathbuf = zrealloc(pathbuf, pathbufsz *= 2);
@@ -271,6 +288,14 @@ addpath(char *s, int l)
 	pathbuf[pathpos++] = *s++;
     pathbuf[pathpos++] = '/';
     pathbuf[pathpos] = '\0';
+#ifdef BROKEN_STAT
+    if (stat(unmeta(pathbuf), &stbuf) || !S_ISDIR(stbuf.st_mode)) {
+	pathbuf[pathpos = oppos] = '\0';
+	return -1;
+    }
+    else
+#endif
+	return 0;
 }
 
 /* stat the filename s appended to pathbuf.  l should be true for lstat,    *
@@ -531,9 +556,12 @@ scanner(Complist q, int shortcircuit)
 			       sr.st_dev != sc.st_dev);
 		    }
 		}
-		if (add) {
-		    addpath(str, l);
-		    if (!closure || !statfullpath("", NULL, 1)) {
+		if (add && !addpath(str,l)) {
+		    if (!closure
+#ifndef BROKEN_STAT
+			    || !statfullpath("", NULL, 1)
+#endif
+		    ) {
 			scanner((q->closure) ? q : q->next, shortcircuit);
 			if (shortcircuit && shortcircuit == matchct)
 			    return;
@@ -650,15 +678,17 @@ scanner(Complist q, int shortcircuit)
 
 	    for (fn = subdirs; fn < subdirs+subdirlen; ) {
 		int l = strlen(fn);
-		addpath(fn, l);
+		int fn_is_dir = !addpath(fn, l);
 		fn += l + 1;
 		memcpy((char *)&errsfound, fn, sizeof(int));
 		fn += sizeof(int);
-		/* scan next level */
-		scanner((q->closure) ? q : q->next, shortcircuit); 
-		if (shortcircuit && shortcircuit == matchct)
-		    return;
-		pathbuf[pathpos = oppos] = '\0';
+		if (fn_is_dir) {
+		    /* scan next level */
+		    scanner((q->closure) ? q : q->next, shortcircuit); 
+		    if (shortcircuit && shortcircuit == matchct)
+			return;
+		    pathbuf[pathpos = oppos] = '\0';
+		}
 	    }
 	    hrealloc(subdirs, subdirlen, 0);
 	}