From 9e06828a7fd61b330ad95490a055eff9e2191933 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Mon, 23 Nov 2015 21:48:49 -0800 Subject: 37208: check for restricted parameter before changing from unset to set Also, return NULL from setnparam() on failure to retrieve value (avoids null-pointer dereference); add some expository comments --- ChangeLog | 5 +++++ Src/params.c | 30 ++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6c40a08e6..8a0f9c075 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2015-11-23 Barton E. Schaefer + * 37208: Src/params.c: check for restricted parameter before + changing from unset to set; return NULL from setnparam() on + failure to retrieve value (avoids null-pointer dereference); + add some expository comments + * 37208: Test/V10parameter.ztst: re-run the "typeset" tests with the private module loaded diff --git a/Src/params.c b/Src/params.c index 602f69f33..4600284f0 100644 --- a/Src/params.c +++ b/Src/params.c @@ -880,6 +880,10 @@ createparam(char *name, int flags) zerr("read-only variable: %s", name); return NULL; } + if ((oldpm->node.flags & PM_RESTRICTED) && isset(RESTRICTED)) { + zerr("%s: restricted", name); + return NULL; + } if (!(oldpm->node.flags & PM_UNSET) || (oldpm->node.flags & PM_SPECIAL)) { oldpm->node.flags &= ~PM_UNSET; if ((oldpm->node.flags & PM_SPECIAL) && oldpm->ename) { @@ -890,10 +894,6 @@ createparam(char *name, int flags) } return NULL; } - if ((oldpm->node.flags & PM_RESTRICTED) && isset(RESTRICTED)) { - zerr("%s: restricted", name); - return NULL; - } pm = oldpm; pm->base = pm->width = 0; @@ -2777,6 +2777,7 @@ assignsparam(char *s, char *val, int flags) if (!v && !(v = getvalue(&vbuf, &t, 1))) { unqueue_signals(); zsfree(val); + /* errflag |= ERRFLAG_ERROR; */ return NULL; } if (flags & ASSPM_WARN_CREATE) @@ -2926,6 +2927,7 @@ assignaparam(char *s, char **val, int flags) if (!(v = fetchvalue(&vbuf, &t, 1, SCANPM_ASSIGNING))) { unqueue_signals(); freearray(val); + /* errflag |= ERRFLAG_ERROR; */ return NULL; } @@ -2988,6 +2990,7 @@ sethparam(char *s, char **val) return NULL; queue_signals(); if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) { + DPUTS(!v, "BUG: assigning to undeclared associative array"); createparam(t, PM_HASHED); checkcreate = isset(WARNCREATEGLOBAL) && locallevel > 0; } else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) && @@ -3000,6 +3003,7 @@ sethparam(char *s, char **val) if (!v) if (!(v = fetchvalue(&vbuf, &t, 1, SCANPM_ASSIGNING))) { unqueue_signals(); + /* errflag |= ERRFLAG_ERROR; */ return NULL; } if (checkcreate) @@ -3061,8 +3065,11 @@ setnparam(char *s, mnumber val) } else if (val.type & MN_INTEGER) { pm->base = outputradix; } - v = getvalue(&vbuf, &t, 1); - DPUTS(!v, "BUG: value not found for new parameter"); + if (!(v = getvalue(&vbuf, &t, 1))) { + DPUTS(!v, "BUG: value not found for new parameter"); + /* errflag |= ERRFLAG_ERROR; */ + return NULL; + } if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0) check_warn_create(v->pm, "numeric"); } @@ -3219,6 +3226,10 @@ unsetparam_pm(Param pm, int altflag, int exp) * * This could usefully be made type-specific, but then we need * to be more careful when calling the unset method directly. + * + * The "exp"licit parameter should be nonzero for assignments and the + * unset command, and zero for implicit unset (e.g., end of scope). + * Currently this is used only by some modules. */ /**/ @@ -5105,8 +5116,11 @@ freeparamnode(HashNode hn) { Param pm = (Param) hn; - /* Since the second flag to unsetfn isn't used, I don't * - * know what its value should be. */ + /* The second argument of unsetfn() is used by modules to + * differentiate "exp"licit unset from implicit unset, as when + * a parameter is going out of scope. It's not clear which + * of these applies here, but passing 1 has always worked. + */ if (delunset) pm->gsu.s->unsetfn(pm, 1); zsfree(pm->node.nam); -- cgit 1.4.1