ast icon indicating copy to clipboard operation
ast copied to clipboard

Remove the magic `A__z` environment var

Open krader1961 opened this issue 6 years ago • 5 comments

While fixing lint in the env_init() function I noticed the magic A__z env var. This is a poorly documented mechanism for passing the properties of exported vars to subshells. I can't find any documentation of this in source code comments, the man page, or The Kornshell book. A google search turns up very few mentions of this mechanism. Two that I found:

http://webcache.googleusercontent.com/search?q=cache:PK89YUimZ0gJ:paul.herger.at/knowhow/kornshell/a_z+&cd=1&hl=en&ct=clnk&gl=us https://unix.stackexchange.com/questions/66627/is-there-anyway-to-set-a-readonly-environment-variable

I propose documenting this as deprecated and removing this mechanism in the next major release. Not least because it only works when a ksh process directly runs another ksh process. If you have an intermediate process that modifies the environment, and which doesn't know about this magic env var, and spawns a ksh subshell then it's possible for bad things to happen. If this isn't removed it needs to be clearly documented.

Be honest, how many of you knew about this mechanism?

krader1961 avatar Jul 01 '18 03:07 krader1961

I had not heard about this but it seems necessary as otherwise, if I create a readonly variable, how can we make sure it stays readonly in a normal subshell at all:

typeset -r TMP=blah
( set | grep A__; echo $TMP ${@TMP} )

Interestingly, the above produces just:

blah typeset -r

No sign of the A__z variable???

Plus, setting it doesn't affect my $TMP variable either:

( A__z=xx; echo $TMP ${@TMP}; echo $A__z )

blah typeset -r
xx

dannyweldon avatar Jul 04 '18 13:07 dannyweldon

@dannyweldon That's because it only affects attributes of exported vars. You didn't export TMP. And it doesn't show up in the output of set because it is a magic var added to the end of the list of environment vars when an external command is run. The set won't show it. Try this instead:

$ typeset -rx TMP=blah
$ env | grep TMP
TMP=blah
TMPDIR=/var/folders/6t/t624jkmx7cbb9t35vzvjwj180000gn/T/
A__z="*SHLVL=! TMP
$ ksh -c 'TMP=foo'
ksh: TMP: is read only

I'll have to check if this mechanism affects (...) subshells. If it does then it definitely would need to be retained and documented.

krader1961 avatar Jul 04 '18 16:07 krader1961

I confirmed that disabling the magic A__z var does not affect (...) subshells. Which makes sense since they don't exec an external program. They are simply a forked clone of the current process and thus have all of its state. Note that all I did was rename the var WTF for the test. Note that the special read-only status of X is no longer honored by an explicit ksh invocation but is still honored by the (...) construct:

$ typeset -r X=yes
$ ( env|grep A__z; env | grep WTF; X=no )
WTF=F*COMP_KEY=F*COMP_POINT=F*COMP_TYPE="*SHLVL
src/cmd/ksh93/ksh: X: is read only
$ ksh -c 'X=no'

krader1961 avatar Jul 05 '18 01:07 krader1961

I knew about this undocumented misfeature because it bit me while developing modernish. Somehow relaunching the installer to use the user's chosen shell failed if ksh was involved. Took months to figure out what was going on.

As @krader1961 says, this has nothing to do with subshells at all, it's trying to make sure that newly initialised ksh processes (which aren't subshells) inherit exported readonly variables as readonly.

Which really seems like a Bad Idea™. There is no way to secure that A__z variable while in the environment since the environment has no concept of readonly. So there is nothing to stop any other process from manipulating that A__z environment variable and influencing/breaking the execution of any ksh scripts started as child processes. This is a potential attack vector.

The only variables that should ever be readonly should be those declared readonly by the current script itself. Yes, please remove this misfeature.

McDutchie avatar Feb 20 '19 10:02 McDutchie

While trying to resolve #1038 I noticed this block of code:

https://github.com/att/ast/blob/80e8b60322fdc2214b887e4a8a4f0e32ab7e1266/src/cmd/ksh93/sh/init.c#L1977

The problem is that it presumes that NV_INTEGER can be encoded in a char. See the (flag & NV_INTEGER) a few lines later in the code. That happens to be true but is a fragile assumption since there is no comment anywhere in the code that explains the special status of those low order four bits of "nvflag" and associated #define symbols.

krader1961 avatar Apr 29 '19 05:04 krader1961