[PATCH] Improve compinit security

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

This was discussed recently on zsh-security and also way back in 2015
in workers/36685.  It does not address the emulation mode issue
mentioned last January in workers/47857 and replies.

This accomplishes a few of the things mentioned in 36685.  For one,
it's now possible to use "autoload -r" to assure that functions are
loaded from the locations where compinit found them, which was not
supported in 2015.  Authors of plugin managers and other add-ons that
re-run compinit should take note.

For another, with the changes herein, "compinit -i" attempts to update
the fpath to remove any insecure directories (unless the -C option
loads an existing dump file).  Unfortunately this further complicates
the emulation mode issue.

Finally, this addresses a long-standing confusion about the meaning of
the "Ignore insecure X and continue?" prompt.  Prior to this patch, it
had the same effect as "compinit -u", that is, it ignored the result
of compaudit.  With the patch, it acts like "compinit -i" and ignores
(skips autoloading of) the suspect files and/or directories.  I
believe this change was discussed at least once (long ago) and
rejected because of the difference in behavior, but I can't find the
reference and it seems unlikely that anyone is frequently encountering
this prompt.

diff --git a/Completion/compinit b/Completion/compinit
index 1f2e7c634..5cb527fac 100644
--- a/Completion/compinit
+++ b/Completion/compinit
@@ -329,7 +329,7 @@ compdef() {
     # and probably do autoloading.
 
     func="$1"
-    [[ -n "$autol" ]] && autoload -Uz "$func"
+    [[ -n "$autol" ]] && autoload -rUz "$func"
     shift
 
     case "$type" in
@@ -451,7 +451,7 @@ typeset _i_wdirs _i_wfiles
 _i_wdirs=()
 _i_wfiles=()
 
-autoload -Uz compaudit
+autoload -RUz compaudit
 if [[ -n "$_i_check" ]]; then
   typeset _i_q
   if ! eval compaudit; then
@@ -467,19 +467,17 @@ Ignore insecure $_i_q and continue [y] or abort compinit [n]? "; then
 
           return 1
         fi
-        _i_wfiles=()
-	_i_wdirs=()
-      else
-        (( $#_i_wfiles )) && _i_files=( "${(@)_i_files:#(${(j:|:)_i_wfiles%.zwc})}"  )
-        (( $#_i_wdirs ))  && _i_files=( "${(@)_i_files:#(${(j:|:)_i_wdirs%.zwc})/*}" )
       fi
+      fpath=(${fpath:|_i_wdirs})
+      (( $#_i_wfiles )) && _i_files=( "${(@)_i_files:#(${(j:|:)_i_wfiles%.zwc})}"  )
+      (( $#_i_wdirs ))  && _i_files=( "${(@)_i_files:#(${(j:|:)_i_wdirs%.zwc})/*}" )
     fi
     typeset -g _comp_secure=yes
   fi
 fi
 
 # Make sure compdump is available, even if we aren't going to use it.
-autoload -Uz compdump compinstall
+autoload -RUz compdump compinstall
 
 # If we have a dump file, load it.
 
@@ -538,7 +536,7 @@ if [[ -z "$_i_done" ]]; then
 	fi
 	;;
       (\#autoload)
-	autoload -Uz "$_i_line[@]" ${_i_name}
+	autoload -rUz "$_i_line[@]" ${_i_name}
 	[[ "$_i_line" != \ # ]] && _compautos[${_i_name}]="$_i_line"
 	;;
       esac
@@ -570,6 +568,6 @@ if [[ ${_i_line[2]} = expand-or-complete ]] &&
 fi
 
 unfunction compinit compaudit
-autoload -Uz compinit compaudit
+autoload -RUz compinit compaudit
 
 return 0
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index f85293ac7..b968f20dc 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -190,7 +190,7 @@ tt(compinit) will ask if the completion system should really be used.  To
 avoid these tests and make all files found be used without asking, use the
 option tt(-u), and to make tt(compinit) silently ignore all insecure files
 and directories use the option tt(-i).  This security check is skipped
-entirely when the tt(-C) option is given.
+entirely when the tt(-C) option is given, provided the dumpfile exists.
 
 findex(compaudit)
 The security check can be retried at any time by running the function
diff --git a/NEWS b/NEWS
index 1e9926c05..832a27e36 100644
--- a/NEWS
+++ b/NEWS
@@ -4,25 +4,8 @@ CHANGES FROM PREVIOUS VERSIONS OF ZSH
 
 Note also the list of incompatibilities in the README file.
 
-Changes since 5.8
------------------
-
-CVE-2021-45444: Some prompt expansion sequences, such as %F, support
-'arguments' which are themselves expanded in case they contain colour
-values, etc. This additional expansion would trigger PROMPT_SUBST
-evaluation, if enabled. This could be abused to execute code the user
-didn't expect. e.g., given a certain prompt configuration, an attacker
-could trick a user into executing arbitrary code by having them check
-out a Git branch with a specially crafted name.
-
-This is fixed in the shell itself by no longer performing PROMPT_SUBST
-evaluation on these prompt-expansion arguments.
-
-Users who are concerned about an exploit but unable to update their
-binaries may apply the partial work-around described in the file
-Etc/CVE-2021-45444-VCS_Info-workaround.patch included with the shell
-source. [ Reported by RyotaK <security@xxxxxxxxx>. Additional thanks to
-Marc Cornellà <hello@xxxxxxxxxxxxx>. ]
+Changes since 5.8.1
+-------------------
 
 When unsetting a hash element, the string enclosed in square brackets is
 interpreted literally after any normal command-line-argument expansions.
@@ -55,6 +38,11 @@ The option CLOBBER_EMPTY was added to enable the overwrite behaviour
 of CLOBBER for empty files only. It is disabled by default.
 
 The compinit function learnt a -w option to explain why compdump runs.
+When run without the -i or -u options and compaudit discovers security
+issues, answering "y" to the "Ignore insecure ..." prompt removes the
+insecure elements (like the -i option) where previously it ignored the
+result (thus formerly like the -u option).  Further, removing those
+elements includes dropping directories from the $fpath array.
 
 The zsh/datetime module's strftime builtin learnt an -n option to omit
 the trailing newline when printing a formatted time.
@@ -70,6 +58,32 @@ attributes such as numeric type, sorting, and padding are retained until
 the parameter is explicitly unset or a conflicting value is assigned.
 This is similar to default behavior of bash and ksh.
 
+The "jobs" command and "$jobstates" and related parameters can report on
+parent shell jobs even in subshells.  This is a snapshot of the parent
+state, frozen at the point the subshell started.  However, if a subshell
+starts its own background jobs, the parent state is discarded in order
+to report on those new jobs.
+
+Changes since 5.8
+-----------------
+
+CVE-2021-45444: Some prompt expansion sequences, such as %F, support
+'arguments' which are themselves expanded in case they contain colour
+values, etc. This additional expansion would trigger PROMPT_SUBST
+evaluation, if enabled. This could be abused to execute code the user
+didn't expect. e.g., given a certain prompt configuration, an attacker
+could trick a user into executing arbitrary code by having them check
+out a Git branch with a specially crafted name.
+
+This is fixed in the shell itself by no longer performing PROMPT_SUBST
+evaluation on these prompt-expansion arguments.
+
+Users who are concerned about an exploit but unable to update their
+binaries may apply the partial work-around described in the file
+Etc/CVE-2021-45444-VCS_Info-workaround.patch included with the shell
+source. [ Reported by RyotaK <security@xxxxxxxxx>. Additional thanks to
+Marc Cornellà <hello@xxxxxxxxxxxxx>. ]
+
 Changes from 5.7.1-test-3 to 5.8
 --------------------------------
 
diff --git a/README b/README
index 353667289..c27d6881a 100644
--- a/README
+++ b/README
@@ -33,6 +33,10 @@ details, see the documentation.
 Incompatibilities since 5.8
 ---------------------------
 
+compinit: A "y" response to the "Ignore ... and continue?" prompt removes
+insecure elements from the set of completion functions, where previously
+it ignored the compaudit result and included all elements.
+
 PROMPT_SUBST expansion is no longer performed on arguments to prompt-
 expansion sequences such as %F.