Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the magic A__z environment var #637

Open
krader1961 opened this issue Jul 1, 2018 · 5 comments
Open

Remove the magic A__z environment var #637

krader1961 opened this issue Jul 1, 2018 · 5 comments

Comments

@krader1961
Copy link
Contributor

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?

@dannyweldon
Copy link

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

@krader1961
Copy link
Contributor Author

@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
Copy link
Contributor Author

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'

@McDutchie
Copy link
Contributor

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.

@krader1961
Copy link
Contributor Author

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

int flag = *(unsigned char *)cp - ' ';

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.

McDutchie added a commit to modernish/modernish that referenced this issue Jul 8, 2019
DEFPATH is exported for efficiency during installation, so for
ksh93 compatibility it cannot be made read-only.
Ref.: att/ast#637 (it bit me again!)

bin/modernish:
- Make DEFPATH read-only in the readonly command that's uncommented
  by the installer. Fixes ksh93 incompat introduced by b41a960.
McDutchie added a commit to modernish/modernish that referenced this issue Jul 9, 2019
DEFPATH is exported for efficiency during installation, so for
ksh93 compatibility it cannot be made read-only.
Ref.: att/ast#637 (it bit me again!)

bin/modernish:
- Make DEFPATH read-only in the readonly command that's uncommented
  by the installer. Fixes ksh93 incompat introduced by b41a960.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants