From 7954855f21145112599e4a66ae199693e4313453 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Wed, 21 Apr 2021 03:56:03 +0100
Subject: [PATCH] Don't import/export readonly attribute via magic A__z env var

While automagically importing/exporting ksh variable attributes via
the environment is probably a misfeature in general (now disabled
for POSIX standard mode), doing so with the readonly attribute is
particularly problematic. Scripts can take into account the
possibility of importing unwanted attributes by unsetting or
typesetting variables before using them. But there is no way for a
script to get rid of an unwanted imported readonly variable. This
is a possible attack vector with no possible mitigation.

This commit blocks both the import and the export of the readonly
attribute through the environment. I consider it a security fix.

src/cmd/ksh93/sh/init.c: env_import_attributes():
- Clear NV_RDONLY from imported attributes before applying them.

src/cmd/ksh93/sh/name.c: sh_envgen():
- Remove NV_RDONLY from bitmask defining attributes to export.
---
 NEWS                        | 3 +++
 src/cmd/ksh93/COMPATIBILITY | 3 +++
 src/cmd/ksh93/sh/init.c     | 3 +++
 src/cmd/ksh93/sh/name.c     | 2 +-
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 92ce30a42..9868bedc7 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
   3. The -c/--call, -n/--name and -s/--standard options matched all variable
      names provided by 'getconf -a', even if none were actual matches.
 
+- The readonly attribute of ksh variables is no longer imported from
+  or exported to other ksh shell instances through the environment.
+
 2021-04-16:
 
 - Fixed a bug in emacs mode: after using tab completion to complete the name
diff --git a/src/cmd/ksh93/COMPATIBILITY b/src/cmd/ksh93/COMPATIBILITY
index 5d6f267b0..a4e1c7705 100644
--- a/src/cmd/ksh93/COMPATIBILITY
+++ b/src/cmd/ksh93/COMPATIBILITY
@@ -129,6 +129,9 @@ For more details, see the NEWS file and for complete details, see the git log.
 	To invoke a possible external command at that path, you can still use
 	a non-canonical path, e.g.: /opt//ast/bin/cat or /opt/ast/./bin/cat
 
+24.	The readonly attribute of ksh variables is no longer imported from
+	or exported to other ksh shell instances through the environment.
+
 ____________________________________________________________________________
 
 		KSH-93 VS. KSH-88
diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c
index 97ca10153..852a14394 100644
--- a/src/cmd/ksh93/sh/init.c
+++ b/src/cmd/ksh93/sh/init.c
@@ -1986,6 +1986,9 @@ static void env_import_attributes(Shell_t *shp, char *next)
 					size--;
 				}
 			}
+			flag &= ~NV_RDONLY;	/* refuse to import readonly attribute */
+			if(!flag)
+				continue;
 			nv_newattr(np,flag|NV_IMPORT|NV_EXPORT,size);
 		}
 	}
diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c
index b77e82ff3..9b571bd8a 100644
--- a/src/cmd/ksh93/sh/name.c
+++ b/src/cmd/ksh93/sh/name.c
@@ -2241,7 +2241,7 @@ char **sh_envgen(void)
 	/* Export variable attributes into env var named by e_envmarker, unless POSIX mode is on */
 	cp = data.attval = strcopy(*data.argnam,e_envmarker);
 	if(!sh_isoption(SH_POSIX))
-		nv_scan(shp->var_tree, attstore,&data,0,(NV_RDONLY|NV_UTOL|NV_LTOU|NV_RJUST|NV_LJUST|NV_ZFILL|NV_INTEGER));
+		nv_scan(shp->var_tree, attstore,&data,0,(NV_UTOL|NV_LTOU|NV_RJUST|NV_LJUST|NV_ZFILL|NV_INTEGER));
 	*data.attval = 0;
 	if(cp!=data.attval)
 		data.argnam++;