zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <opk@zsh.org>
To: Ray Andrews <rayandrews@eastlink.ca>
Cc: zsh-workers@zsh.org, Thomas Lauer <thomas.lauer@virgin.net>
Subject: Re: special characters in file names issue
Date: Sun, 12 Nov 2023 00:32:38 +0100	[thread overview]
Message-ID: <34569-1699745558.781786@JGMx.WsJC.lR_9> (raw)
In-Reply-To: <fa2af3a1-ff04-44dd-890f-872fb6e5f836@eastlink.ca>

[ 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.


           reply	other threads:[~2023-11-11 23:33 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <fa2af3a1-ff04-44dd-890f-872fb6e5f836@eastlink.ca>]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34569-1699745558.781786@JGMx.WsJC.lR_9 \
    --to=opk@zsh.org \
    --cc=rayandrews@eastlink.ca \
    --cc=thomas.lauer@virgin.net \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).