Re: BUG: crafting SHELLOPTS and PS4 allows to run arbitrary programs in setuid binaries using system

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Wed, 28 Sep 2016 12:37:32 +0200
Simon Ruderich <simon@xxxxxxxxxxxx> wrote:
> I think the test should be changed to getuid() != geteuid() or
> similar to trigger only in setuid cases.

That seems to catch all the cases where there might be surprises.

Oliver pointed out there's already the PRIVILEGED option for this
purpose, which is set up before variables.  It's definition is a little
different,

    opts[PRIVILEGED] = (getuid() != geteuid() || getgid() != getegid());

but that's probably good enough.

pws

diff --git a/Src/params.c b/Src/params.c
index 87586a2..8271a8b 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -333,7 +333,7 @@ IPDEF6("TRY_BLOCK_ERROR", &try_errflag, varinteger_gsu),
 IPDEF6("TRY_BLOCK_INTERRUPT", &try_interrupt, varinteger_gsu),
 
 #define IPDEF7(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL},BR((void *)B),GSU(varscalar_gsu),0,0,NULL,NULL,NULL,0}
-#define IPDEF7R(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL|PM_DONTIMPORT_ROOT},BR((void *)B),GSU(varscalar_gsu),0,0,NULL,NULL,NULL,0}
+#define IPDEF7R(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL|PM_DONTIMPORT_SUID},BR((void *)B),GSU(varscalar_gsu),0,0,NULL,NULL,NULL,0}
 #define IPDEF7U(A,B) {{NULL,A,PM_SCALAR|PM_SPECIAL|PM_UNSET},BR((void *)B),GSU(varscalar_gsu),0,0,NULL,NULL,NULL,0}
 IPDEF7("OPTARG", &zoptarg),
 IPDEF7("NULLCMD", &nullcmd),
@@ -705,8 +705,8 @@ static int dontimport(int flags)
     /* If value already exported */
     if (flags & PM_EXPORTED)
 	return 1;
-    /* If security issue when exporting as root */
-    if ((flags & PM_DONTIMPORT_ROOT) && (!getuid() || !geteuid()))
+    /* If security issue when importing and running with some privilege */
+    if ((flags & PM_DONTIMPORT_SUID) && isset(PRIVILEGED))
 	return 1;
     /* OK to import */
     return 0;
diff --git a/Src/zsh.h b/Src/zsh.h
index 052d754..79747d6 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1802,7 +1802,7 @@ struct tieddata {
 #define PM_ZSHSTORED	(1<<18) /* function stored in zsh form              */
 
 /* Remaining flags do not correspond directly to command line arguments */
-#define PM_DONTIMPORT_ROOT (1<<19) /* do not import if running as root */
+#define PM_DONTIMPORT_SUID (1<<19) /* do not import if running setuid */
 #define PM_SINGLE       (1<<20) /* special can only have a single instance  */
 #define PM_LOCAL	(1<<21) /* this parameter will be made local        */
 #define PM_SPECIAL	(1<<22) /* special builtin parameter                */