[PATCH] Re: crash with nameref and local argv

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Tue, Nov 4, 2025 at 2:33 PM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> There's some other inconsistency in validate_refname() that I have
> just noticed.  For example, it begins by rejecting all parameters
> beginning with a digit, but then later checks would allow a reference
> to the special parameter $0 if things got that far.

The attached patch redoes valid_nameref() to remove the inconsistency
and to ban upscope references to names that refer to the positional
parameters of the caller.

On Sun, Nov 9, 2025 at 10:56 PM Stephane Chazelas <stephane@xxxxxxxxxxxx> wrote:
>
> AFAICT, all shells (Bourne-like ones, but also csh or rc)
> treat ${01} the same as $1 (and ${010} the same as $10, not $8).
> That's even an explicit POSIX requirement:

Given this, the "remove inconsistency" part of the patch syntactically
allows named references to $00, $000, etc. (rather than only $0), and
to current-scope positionals (other than @ * and #), but there is
presently a bug with scalar slices when the referent is ! ? $
(although that seems of little utility) and with dereferencing when
the referent is 00, 1, 01, 2, etc.  Most tests presently just pass
this as success (rather than being xfail) and it's possible it would
be better to revert to making the latter invalid, but I wanted to get
this much in place before digging into Philippe's other patches.

diff --git a/Src/params.c b/Src/params.c
index b76fb8a6b..ccb73c9b6 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1309,7 +1309,6 @@ isident(char *s)
     /* Require balanced [ ] pairs with something between */
     if (!(ss = parse_subscript(++ss, 1, ']')))
 	return 0;
-    untokenize(s);
     return !ss[1];
 }
 
@@ -3247,8 +3246,8 @@ assignsparam(char *s, char *val, int flags)
 	return NULL;
     }
     if (*val && (v->pm->node.flags & PM_NAMEREF)) {
-	if (!valid_refname(val)) {
-	    zerr("invalid variable name: %s", val);
+	if (!valid_refname(val, v->pm->node.flags)) {
+	    zerr("invalid name reference: %s", val);
 	    zsfree(val);
 	    unqueue_signals();
 	    errflag |= ERRFLAG_ERROR;
@@ -6505,30 +6504,48 @@ upscope_upper(Param pm, int reflevel)
 
 /**/
 static int
-valid_refname(char *val)
+valid_refname(char *val, int flags)
 {
-    char *t = itype_end(val, INAMESPC, 0);
+    char *t;
 
-    if (idigit(*val))
-	return 0;
-    if (*t != 0) {
-	if (*t == '[') {
-	    tokenize(t = dupstring(t+1));
-	    while ((t = parse_subscript(t, 0, ']')) && *t++ == Outbrack) {
-		if (*t == Inbrack)
-		    ++t;
-		else
-		    break;
-	    }
-	    if (t && *t) {
-		/* zwarn("%s: stuff after subscript: %s", val, t); */
-		t = NULL;
-	    }
-	} else if (t[1] || !(*t == '!' || *t == '?' ||
-			     *t == '$' || *t == '-' ||
-			     *t == '0' || *t == '_')) {
-	    /* Skipping * @ # because of doshfunc() implementation */
-	    t = NULL;
+    if (flags & PM_UPPER) {
+	/* Upward reference to positionals is doomed to fail */
+	if (idigit(*val))
+	    return 0;
+	t = itype_end(val, INAMESPC, 0);
+	if ((t - val == 4) &&
+	    (!strncmp(val, "argv", 4) ||
+	     !strncmp(val, "ARGC", 4)))
+	    return 0;
+    } else if (idigit(*val)) {
+	t = val;
+	while (*++t)
+	    if (!idigit(*t))
+		break;
+	if (*t && *t != '[')	/* Need to test Inbrack here too? */
+	    return 0;
+    } else
+	t = itype_end(val, INAMESPC, 0);
+
+    if (t == val) {
+	if (!(*t == '!' || *t == '?' ||
+	      *t == '$' || *t == '-' ||
+	      *t == '_'))
+	    return 0;
+	++t;
+    }
+    if (*t == '[') {
+	/* Another bit of isident() to emulate */
+	tokenize(t = dupstring(t+1));
+	while ((t = parse_subscript(t, 0, ']')) && *t++ == Outbrack) {
+	    if (*t == Inbrack)
+		++t;
+	    else
+		break;
+	}
+	if (t && *t) {
+	    /* zwarn("%s: stuff after subscript: %s", val, t); */
+	    return 0;
 	}
     }
     return !!t;
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index 5d229a94e..29a7af0de 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -531,7 +531,7 @@ F:ksh93 does not implement this either
  unset -n ptr1
  typeset -n ptr1='not[2]good'
 1:invalid nameref
-*?*invalid variable name: not\[2\]good
+*?*invalid name reference: not\[2\]good
 
  unset -n ptr1
  unset hash
@@ -1181,6 +1181,91 @@ F:previously this could create an infinite recursion and crash
 >h2: ref1=f1 ref2=f1 ref3=f1
 >f1: ref1=f1 ref2=f1 ref3=f1
 
+#
+# The following two tests are linked, do not separate
+#
+
+ edgelocal() ( local -n x=$1; typeset -p x; print -r $x )
+ edgeupper() ( local -nu x=$1; typeset -p x; print -r $x )
+ for edge in argv ARGC \@ \* \# 0 1 01 \! \? - _
+ do
+  edgelocal $edge
+  edgelocal "$edge""[1]"
+  edgeupper $edge
+ done
+0:references to builtin specials
+F:Subscripting on 1 01 ! ? - should print first character but do not
+>typeset -n x=argv
+>argv
+>typeset -n x='argv[1]'
+>argv[1]
+>typeset -n x=ARGC
+>1
+>typeset -n x='ARGC[1]'
+>1
+>typeset -n x=0
+>edgelocal
+>typeset -n x='0[1]'
+>e
+>typeset -n x=1
+>
+>typeset -n x='1[1]'
+>
+>typeset -n x=01
+>
+>typeset -n x='01[1]'
+>
+>typeset -n x=!
+>0
+>typeset -n x='![1]'
+>
+>typeset -un x=!
+>0
+>typeset -n x='?'
+>0
+>typeset -n x='?[1]'
+>
+>typeset -un x='?'
+>0
+>typeset -n x=-
+>569X
+>typeset -n x='-[1]'
+>
+>typeset -un x=-
+>569X
+>typeset -n x=_
+>x
+>typeset -n x='_[1]'
+>x
+>typeset -un x=_
+>x
+?edgeupper: invalid name reference: argv
+?edgeupper: invalid name reference: ARGC
+?edgelocal: invalid name reference: @
+?edgelocal: invalid name reference: @[1]
+?edgeupper: invalid name reference: @
+?edgelocal: invalid name reference: *
+?edgelocal: invalid name reference: *[1]
+?edgeupper: invalid name reference: *
+?edgelocal: invalid name reference: #
+?edgelocal: invalid name reference: #[1]
+?edgeupper: invalid name reference: #
+?edgeupper: invalid name reference: 0
+?edgeupper: invalid name reference: 1
+?edgeupper: invalid name reference: 01
+
+ edgelocal \$
+ edgelocal '$[1]'
+ edgeupper \$
+ unfunction edgelocal edgeupper
+0qf:references to $$
+F:$$[1] reference should print the first digit of $$ but prints nothing
+>typeset -n x='$'
+>$$
+>typeset -n x='\$[1]'
+>$$[1]
+>$$
+
 #
 # The following tests are run in interactive mode, using PS1 as an
 # assignable special with side-effects.  This crashed at one time.