Re: special characters in file names issue

[Oliver Kiddle <opk@xxxxxxx>]

[ moved to -workers ]

Ray Andrews wrote:
>
> Another hypothetical:  Since all that was added not too long ago, and since
> Oliver is still kicking and could comment on the issue,  what would be the
> practicalities of Thomas just hacking the code to his own satisfaction?  That
> kind of solution never seems to be suggested but it would seem to be a
> possibility.  Too complicated?  Too dangerous?

Anyone, whether that be Thomas or yourself is always welcome to hack the
code and we can discuss it here on the workers list.

The "surprising" effects Roman pointed to in 29343 worry me more
than the initial problem. Evaluating substitutions in the result of
substitutions doesn't seem too clever from a security standpoint. The
same effect occurs in other places like printf -v and likely every other
builtin that takes a variable name as an argument.

[[ -v ... ]] was primarily added for bash compatibility. bash and ksh93
apply expansion to what follows -v. In ksh93, however no substitutions
are applied for the evaluation of the subscript. In bash, the same
double substitution also occurs but only for very simple cases.
Certainly I've not been able to get a command-substitution through.

The patch below tries the approach of adding a SCANPM_NOSUBST flag
similar to the relatively new SCANPM_NOEXEC. This doesn't entirely help
the original problem where using [[ -v 'FileNameCkSum[$E]' ]] might have
helped. Given that [[ ... ]] is special-cased by the parser, disabling
the first substitution would perhaps be better. This does nothing for
the problem with print -v and while
ary=( 1 2 3 4 ); [[ -v 'ary[$(echo 2)]' ]] produces
an error, [[ -v 'ary[ary[$(echo 2)]]' ]] does not.

While I've included the patch for anyone interested the play with, I
think I would lean in the direction of only passing SCANPM_NOEXEC.

Oliver

diff --git a/Src/params.c b/Src/params.c
index 9f0cbcd67..e77f55104 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -711,7 +711,8 @@ issetvar(char *name)
     int slice;
     char **arr;
 
-    if (!(v = getvalue(&vbuf, &name, 1)) || *name)
+    if (!(v = fetchvalue(&vbuf, &name, 1, SCANPM_NOSUBST|SCANPM_NOEXEC))
+	    || *name)
 	return 0; /* no value or more chars after the variable name */
     if (v->isarr & ~SCANPM_ARRONLY)
 	return v->end > 1; /* for extracted elements, end gives us a count */
@@ -1502,7 +1503,8 @@ getarg(char **str, int *inv, Value v, int a2, zlong *w,
 	    return 0;
 	if (flags & SCANPM_NOEXEC)
 	    opts[EXECOPT] = 0;
-	singsub(&s);
+	if (!(flags & SCANPM_NOSUBST))
+	    singsub(&s);
 	opts[EXECOPT] = exe;
     } else if (rev)
 	remnulargs(s);	/* This is probably always a no-op, but ... */
@@ -2207,7 +2209,8 @@ fetchvalue(Value v, char **pptr, int bracks, int flags)
 	}
 	if (PM_TYPE(pm->node.flags) & (PM_ARRAY|PM_HASHED)) {
 	    /* Overload v->isarr as the flag bits for hashed arrays. */
-	    v->isarr = flags | (isvarat ? SCANPM_ISVAR_AT : 0);
+	    v->isarr = (flags & ~(SCANPM_NOSUBST|SCANPM_NOEXEC)) |
+		    (isvarat ? SCANPM_ISVAR_AT : 0);
 	    /* If no flags were passed, we need something to represent *
 	     * `true' yet differ from an explicit WANTVALS.  Use a     *
 	     * special flag for this case.                             */
diff --git a/Src/zsh.h b/Src/zsh.h
index a0243e98e..8d1cf8f15 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1963,13 +1963,14 @@ struct tieddata {
 				  */
 #define SCANPM_CHECKING   (1<<10) /* Check if set, no need to create */
 #define SCANPM_NOEXEC     (1<<11) /* No command substitutions, etc. */
+#define SCANPM_NOSUBST    (1<<14) /* No substitutions */
 #define SCANPM_NONAMESPC  (1<<12) /* namespace syntax not allowed */
 #define SCANPM_NONAMEREF  (1<<13) /* named references are not followed */
 
 /* "$foo[@]"-style substitution
  * Only sign bit is significant
  */
-#define SCANPM_ISVAR_AT   ((int)(((unsigned int)-1)<<15))
+#define SCANPM_ISVAR_AT   ((int)(((unsigned int)-1)<<16))
 
 /*
  * Flags for doing matches inside parameter substitutions, i.e.