Re: Colon-array variables can crash "sh" emulation

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Fri, 14 Apr 2017 19:31:04 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Apr 14,  7:39pm, Anthony Fletcher wrote:
> } 
> } Playing further I provoked a segmentation fault! This dies:
> }
> }    ARGV0=sh zsh -c 'typeset -U path PATH'
> 
> Well, well, well.
> 
> The special_params array in Src/params.c is divided into two sections,
> parameters available in all emulations and those available only in zsh
> emulation.
> 
> Trouble is the parameters in the first group include numerous colon-array
> scalars that are potentially tied to true-arrays from the second group.
> 
> This normally doesn't cause any issues, because the parameters in the
> second group are initialized only in the appropriate emulation.
> 
> BUT -- the parameters in the first group will still try to read values
> from the linked parameters in the second group if the attributes of the
> first parameter are changed, as by "typeset -U".  Again this is usually
> not a problem because the linked parameters are still not set.
> 
> However, if the linked parameter has been declared scalar, bin_typeset
> will attempt to manipulate it as an array, and wallabies go roaming the
> outback, as one of PWS's comments puts it.
> 
> This will happen with any pair of variables: path/PATH, watch/WATCH, etc.
> 
> However, I'm not sure of the best way to approach fixing this.  My first
> thought was that the linkage should be broken during createparamtable(),
> but that turns out to be rather ugly to do.

As path is not a special at all in this case, the fix must be somewhere
or other in the linkage from PATH to path.

The only robust solution looks like being not let PATH's ename element
point at path.  Otherwise the code following ename has to jump through
hoops to see if the parameters are "really" linked, which is far too
late.

I think we need somehow to keep track of the variables in params.c
defined by IPDEF8 and in sh emulation not set ename or null it out using
the same test that decides if the reference parameters will be special.
The path of least resistance seems to be simply declaring the variables
separately.

The fly in the ointment here is that colonarrsetfn() refuses to fix up
the environment unless the ename element is set.  It's not clear to me
why since I don't think ename can be unset for special tied variables
which are the only use of colonarrsetfn().  If that can go the following
might be good enough.  It needs some more verification using sh emulation
at shell start, though.

pws

diff --git a/Src/params.c b/Src/params.c
index a9683a6..d92dd22 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -361,6 +361,17 @@ IPDEF7("PS3", &prompt3),
 IPDEF7R("PS4", &prompt4),
 IPDEF7("SPROMPT", &sprompt),
 
+#define IPDEF9F(A,B,C,D) {{NULL,A,D|PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT},BR((void *)B),GSU(vararray_gsu),0,0,NULL,C,NULL,0}
+#define IPDEF9(A,B,C) IPDEF9F(A,B,C,0)
+IPDEF9F("*", &pparams, NULL, PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT|PM_READONLY),
+IPDEF9F("@", &pparams, NULL, PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT|PM_READONLY),
+
+/*
+ * This empty row indicates the end of parameters available in
+ * all emulations.
+ */
+{{NULL,NULL,0},BR(NULL),NULL_GSU,0,0,NULL,NULL,NULL,0},
+
 #define IPDEF8(A,B,C,D) {{NULL,A,D|PM_SCALAR|PM_SPECIAL},BR((void *)B),GSU(colonarr_gsu),0,0,NULL,C,NULL,0}
 IPDEF8("CDPATH", &cdpath, "cdpath", 0),
 IPDEF8("FIGNORE", &fignore, "fignore", 0),
@@ -374,17 +385,6 @@ IPDEF8("ZSH_EVAL_CONTEXT", &zsh_eval_context, "zsh_eval_context", PM_READONLY),
 /* MODULE_PATH is not imported for security reasons */
 IPDEF8("MODULE_PATH", &module_path, "module_path", PM_DONTIMPORT|PM_RESTRICTED),
 
-#define IPDEF9F(A,B,C,D) {{NULL,A,D|PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT},BR((void *)B),GSU(vararray_gsu),0,0,NULL,C,NULL,0}
-#define IPDEF9(A,B,C) IPDEF9F(A,B,C,0)
-IPDEF9F("*", &pparams, NULL, PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT|PM_READONLY),
-IPDEF9F("@", &pparams, NULL, PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT|PM_READONLY),
-
-/*
- * This empty row indicates the end of parameters available in
- * all emulations.
- */
-{{NULL,NULL,0},BR(NULL),NULL_GSU,0,0,NULL,NULL,NULL,0},
-
 #define IPDEF10(A,B) {{NULL,A,PM_ARRAY|PM_SPECIAL},BR(NULL),GSU(B),10,0,NULL,NULL,NULL,0}
 
 /*
@@ -424,6 +424,26 @@ IPDEF10("pipestatus", pipestatus_gsu),
 };
 
 /*
+ * Alternative versions of colon-separated path parameters for
+ * sh emulation.  These don't link to the array versions.
+ */
+static initparam special_params_sh[] = {
+IPDEF8("CDPATH", &cdpath, NULL, 0),
+IPDEF8("FIGNORE", &fignore, NULL, 0),
+IPDEF8("FPATH", &fpath, NULL, 0),
+IPDEF8("MAILPATH", &mailpath, NULL, 0),
+IPDEF8("WATCH", &watch, NULL, 0),
+IPDEF8("PATH", &path, NULL, PM_RESTRICTED),
+IPDEF8("PSVAR", &psvar, NULL, 0),
+IPDEF8("ZSH_EVAL_CONTEXT", &zsh_eval_context, NULL, PM_READONLY),
+
+/* MODULE_PATH is not imported for security reasons */
+IPDEF8("MODULE_PATH", &module_path, NULL, PM_DONTIMPORT|PM_RESTRICTED),
+
+{{NULL,NULL,0},BR(NULL),NULL_GSU,0,0,NULL,NULL,NULL,0},
+};
+
+/*
  * Special way of referring to the positional parameters.  Unlike $*
  * and $@, this is not readonly.  This parameter is not directly
  * visible in user space.
@@ -753,9 +773,13 @@ createparamtable(void)
     /* Add the special parameters to the hash table */
     for (ip = special_params; ip->node.nam; ip++)
 	paramtab->addnode(paramtab, ztrdup(ip->node.nam), ip);
-    if (!EMULATION(EMULATE_SH|EMULATE_KSH))
+    if (EMULATION(EMULATE_SH|EMULATE_KSH)) {
+	for (ip = special_params_sh; ip->node.nam; ip++)
+	    paramtab->addnode(paramtab, ztrdup(ip->node.nam), ip);
+    } else {
 	while ((++ip)->node.nam)
 	    paramtab->addnode(paramtab, ztrdup(ip->node.nam), ip);
+    }
 
     argvparam = (Param) &argvparam_pm;
 
@@ -3857,8 +3881,7 @@ colonarrsetfn(Param pm, char *x)
 	*dptr = colonsplit(x, pm->node.flags & PM_UNIQUE);
     else
 	*dptr = mkarray(NULL);
-    if (pm->ename)
-	arrfixenv(pm->node.nam, *dptr);
+    arrfixenv(pm->node.nam, *dptr);
     zsfree(x);
 }