init: fix command injection in bash Tab completion (compgen -W expansion)

.gitignore

@@ -61,6 +61,8 @@
 /testsuite/completion-use-fixture
 /testsuite/completion-home
 /testsuite/cookbook-sandbox
+/testsuite/completion-inject-fixture
+/testsuite/completion-injection-marker
 /doc/build
 /doc/source/version.py
 /doc/demo/*/*.svg

.hunspell.en.dic

@@ -263,6 +263,7 @@ autotools
 availabilities
 avx
 ba
+backtick
 backticks
 badcommand
 baf
@@ -285,6 +286,7 @@ boolvariantname
 boolvr
 bourne
 bugfix
+builtin
 cachebuild
 cacheclear
 cachefile
@@ -341,9 +343,11 @@ comgen
 commandexp
 commandname
 compA
+compadd
 compB
 compat
 compdef
+compgen
 completionhome
 compilerTag
 compopt
@@ -841,6 +845,7 @@ subdir
 subdirectories
 subdirectory
 submodule
+subprocess
 subprojects
 subshell
 substring
@@ -922,6 +927,7 @@ unloadable
 unprefixed
 unsetConf
 unsetModuleDependency
+untrusted
 unsetState
 unsetenv
 unsets
@@ -1617,3 +1623,4 @@ cuda
 lindex
 defineModStartNbProc
 isIcase
+CVE

NEWS.rst

@@ -251,6 +251,12 @@ Modules 5.7.0 (not yet released)
 * Doc: add the :ref:`user-guide` document that explains a selection of
   useful but lesser known features through practical examples and common
   use cases.
+* Init: fix command injection in Bash completion when module names contain
+  shell meta-characters. Completion candidates were passed to ``compgen -W``
+  which evaluates command substitution syntax. (fix `CVE-2026-85013`_ found
+  by AISLE in partnership with Red Hat)
+
+.. _CVE-2026-85013: https://github.com/envmodules/modules/security/advisories/GHSA-8hrw-p88g-qhmg
 
 
 .. _5.6 release notes:

doc/source/devel/testsuite.rst

@@ -46,8 +46,10 @@ DejaGnu tools:
     Drives a real, interactive shell process (via Expect ``spawn``/``send``/
     ``expect``, not just a captured non-interactive run like the other three
     tools) to press Tab against the built shell completion script and check
-    that the resulting candidate list holds the expected module names and
-    option flags. Driven by the :file:`completion.00-init` directory;
+    the resulting candidate list -- both that expected module names/option
+    flags show up, and that a candidate word built from untrusted text (a
+    module name, ``LOADEDMODULES``, ``MODULEPATH``, ...) can never reach a
+    shell expansion step. Driven by the :file:`completion.00-init` directory;
     currently covers bash, zsh, fish and tcsh, see `completion.00-init
     layout`_.
 
@@ -181,9 +183,9 @@ shell-specific in a way none of the other three tools are:
   :file:`init/zsh-functions/_module`, :file:`init/fish_completion` and
   :file:`init/tcsh_completion` scripts, a clean fixture modulepath), the
   shell-agnostic assert procedures (``completion_assert_contains``,
-  ``completion_assert_not_contains``, ``completion_assert_eq``), and the
-  clean baseline environment/``save_test_env`` checkpoint, exactly as for
-  the other tools.
+  ``completion_assert_not_contains``, ``completion_assert_eq``,
+  ``completion_assert_no_exec``), and the clean baseline
+  environment/``save_test_env`` checkpoint, exactly as for the other tools.
 - ``0NN-<shell>-procs.exp`` defines one ``completion_<shell>_start`` /
   ``completion_<shell>_raw`` / ``completion_<shell>_list`` /
   ``completion_<shell>_inline`` / ``completion_<shell>_close`` set per

init/bash_completion.in

@@ -2,17 +2,30 @@
 #
 # Bash commandline completion
 #
+_module_comgen_words() {
+    local -a words
+    local val
+    # split candidate list on IFS without triggering shell expansion (command
+    # substitution, arithmetic, ...): candidate words may come from module
+    # names read off disk, which must never be treated as executable code
+    IFS=$' \t\n' read -r -d '' -a words <<<"$1"
+    for val in "${words[@]}"; do
+        case "$val" in
+            "$2"*) COMPREPLY[${#COMPREPLY[@]}]="$val" ;;
+        esac
+    done
+}
+
 _module_comgen_words_and_files() {
-    local k=0
-    local setnospace=1
+    local start=${#COMPREPLY[@]} i
+    _module_comgen_words "$1" "$2"
     # do not append space to word completed if it is a directory (ends with /)
-    for val in $(compgen -W "$1" -- "$2"); do
-        if [ $setnospace -eq 1 ] && [ "${val: -1:1}" = '/' ]; then
+    for ((i = start; i < ${#COMPREPLY[@]}; i++)); do
+        if [ "${COMPREPLY[i]: -1:1}" = '/' ]; then
             # Bash >=4.0 is required for compopt
             type compopt &>/dev/null && compopt -o nospace
-            setnospace=0
+            break
         fi
-        COMPREPLY[k++]="$val"
     done
 }
 
@@ -64,7 +77,7 @@ _module_long_arg_list() {
             _module_comgen_words_and_files "$(_module_not_yet_loaded "$cur")" "$cur"
             break;;
         rm|delete|remove|unload|switch|swap)
-            COMPREPLY=( $(IFS=: compgen -W "${LOADEDMODULES}" -- "$cur") )
+            _module_comgen_words "${LOADEDMODULES//:/ }" "$cur"
             break;;
         esac
     done
@@ -85,19 +98,19 @@ _module() {
     list)  COMPREPLY=( $(compgen -W "@comp_list_opts@" -- "$cur") );;
     savelist)  COMPREPLY=( $(compgen -W "@comp_savelist_opts@" -- "$cur") );;
     stashlist)  COMPREPLY=( $(compgen -W "@comp_stashlist_opts@" -- "$cur") );;
-    stashpop)  COMPREPLY=( $(compgen -W "@comp_stashpop_opts@ $(_module_stashlist)" -- "$cur") );;
+    stashpop)  _module_comgen_words "@comp_stashpop_opts@ $(_module_stashlist)" "$cur";;
     stashshow|stashrm)
-                    COMPREPLY=( $(compgen -W "$(_module_stashlist)" -- "$cur") );;
+                    _module_comgen_words "$(_module_stashlist)" "$cur";;
     clear)  COMPREPLY=( $(compgen -W "@comp_clear_opts@" -- "$cur") );;
-    restore)  COMPREPLY=( $(compgen -W "@comp_restore_opts@ $(_module_savelist)" -- "$cur") );;
+    restore)  _module_comgen_words "@comp_restore_opts@ $(_module_savelist)" "$cur";;
     save|saveshow|describe|saverm|disable|is-saved)
-                    COMPREPLY=( $(compgen -W "$(_module_savelist)" -- "$cur") );;
+                    _module_comgen_words "$(_module_savelist)" "$cur";;
     rm|delete|remove|unload)
-                    COMPREPLY=( $(compgen -W "@comp_unload_opts@ ${LOADEDMODULES//:/ }" -- "$cur") );;
-    switch|swap)    COMPREPLY=( $(compgen -W "@comp_load_opts@ ${LOADEDMODULES//:/ }" -- "$cur") );;
-    unuse|is-used)  COMPREPLY=( $(IFS=: compgen -W "${MODULEPATH}" -- "$cur") );;
+                    _module_comgen_words "@comp_unload_opts@ ${LOADEDMODULES//:/ }" "$cur";;
+    switch|swap)    _module_comgen_words "@comp_load_opts@ ${LOADEDMODULES//:/ }" "$cur";;
+    unuse|is-used)  _module_comgen_words "${MODULEPATH//:/ }" "$cur";;
     use)    case "$cur" in
-                -*) COMPREPLY=( $(compgen -W "@comp_use_opts@" -- "$cur") );;
+                -*) _module_comgen_words "@comp_use_opts@" "$cur";;
                 *)  ;;               # let readline handle the completion
             esac;;
     -a|--append|cachebuild)   ;;               # let readline handle the completion
@@ -161,19 +174,19 @@ if type -t ml >/dev/null; then
         list)  COMPREPLY=( $(compgen -W "@comp_list_opts@" -- "$cur") );;
         savelist)  COMPREPLY=( $(compgen -W "@comp_savelist_opts@" -- "$cur") );;
         stashlist)  COMPREPLY=( $(compgen -W "@comp_stashlist_opts@" -- "$cur") );;
-        stashpop)  COMPREPLY=( $(compgen -W "@comp_stashpop_opts@ $(_module_stashlist)" -- "$cur") );;
+        stashpop)  _module_comgen_words "@comp_stashpop_opts@ $(_module_stashlist)" "$cur";;
         stashshow|stashrm)
-                    COMPREPLY=( $(compgen -W "$(_module_stashlist)" -- "$cur") );;
+                    _module_comgen_words "$(_module_stashlist)" "$cur";;
         clear)  COMPREPLY=( $(compgen -W "@comp_clear_opts@" -- "$cur") );;
-        restore)  COMPREPLY=( $(compgen -W "@comp_restore_opts@ $(_module_savelist)" -- "$cur") );;
+        restore)  _module_comgen_words "@comp_restore_opts@ $(_module_savelist)" "$cur";;
         save|saveshow|describe|saverm|disable|is-saved)
-                        COMPREPLY=( $(compgen -W "$(_module_savelist)" -- "$cur") );;
+                        _module_comgen_words "$(_module_savelist)" "$cur";;
         rm|delete|remove|unload)
-                        COMPREPLY=( $(compgen -W "@comp_unload_opts@ ${LOADEDMODULES//:/ }" -- "$cur") );;
-        switch|swap)    COMPREPLY=( $(compgen -W "@comp_load_opts@ ${LOADEDMODULES//:/ }" -- "$cur") );;
-        unuse|is-used)  COMPREPLY=( $(IFS=: compgen -W "${MODULEPATH}" -- "$cur") );;
+                        _module_comgen_words "@comp_unload_opts@ ${LOADEDMODULES//:/ }" "$cur";;
+        switch|swap)    _module_comgen_words "@comp_load_opts@ ${LOADEDMODULES//:/ }" "$cur";;
+        unuse|is-used)  _module_comgen_words "${MODULEPATH//:/ }" "$cur";;
         use)    case "$cur" in
-                    -*) COMPREPLY=( $(compgen -W "@comp_use_opts@" -- "$cur") );;
+                    -*) _module_comgen_words "@comp_use_opts@" "$cur";;
                     *)  ;;               # let readline handle the completion
                 esac;;
         -a|--append|cachebuild)   ;;               # let readline handle the completion
@@ -218,14 +231,14 @@ if type -t ml >/dev/null; then
                         for i in ${LOADEDMODULES//:/ }; do
                             loaded_modules+="-${i} "
                         done
-                        COMPREPLY=( "${COMPREPLY[@]}" $(compgen -W "@comp_load_opts@ $loaded_modules" -- "$cur") );;
+                        _module_comgen_words "@comp_load_opts@ $loaded_modules" "$cur";;
                 *)       _module_comgen_words_and_files "@comp_load_opts@ $(_module_not_yet_loaded "$cur")" "$cur"
                         COMPREPLY=( "${COMPREPLY[@]}" $(compgen -W "@comp_opts@ @comp_cmds@" -- "$cur") )
                         loaded_modules=""
                         for i in ${LOADEDMODULES//:/ }; do
                             loaded_modules+="-${i} "
                         done
-                        COMPREPLY=( "${COMPREPLY[@]}" $(compgen -W "@comp_load_opts@ $loaded_modules" -- "$cur") );;
+                        _module_comgen_words "@comp_load_opts@ $loaded_modules" "$cur";;
                 esac
             fi;;
         esac

testsuite/completion.00-init/005-init_ts.exp

@@ -121,4 +121,8 @@ set out [open $tcshcompletion w]
 puts -nonewline $out $tcshcompletion_content
 close $out
 
+# scratch area for modulefiles created on the fly by completion tests
+set injectdir "$env(TESTSUITEDIR)/completion-inject-fixture"
+file delete -force $injectdir
+
 # vim:set tabstop=3 shiftwidth=3 expandtab autoindent:

testsuite/completion.00-init/006-procs.exp

@@ -130,4 +130,20 @@ proc completion_assert_eq {got expected} {
    }
 }
 
+# assert a marker file was not created (used to prove no shell code
+# injected through a candidate word got executed); the test label is built
+# from the cmdline last passed to completion_<shell>_list plus the marker
+# path
+proc completion_assert_no_exec {markerpath} {
+   global completion_last_cmdline
+
+   set label "'$completion_last_cmdline' completion does not execute injected code ($markerpath)"
+   if {[file exists $markerpath]} {
+      fail "$label (marker file was created: injected code ran)"
+      file delete -force $markerpath
+   } else {
+      pass $label
+   }
+}
+
 # vim:set tabstop=3 shiftwidth=3 expandtab autoindent:

testsuite/completion.00-init/021-bash.exp

@@ -10,7 +10,7 @@
 #   Authors:		Xavier Delaruelle, xavier.delaruelle@cea.fr
 #
 #   Description:	Testuite testsequence
-#   Command:		avail, load, unload, ml, use, unuse, restore
+#   Command:		avail, load, unload, ml, use, unuse, restore, switch
 #   Sub-Command:
 #
 #   Comment:	%C{
@@ -26,7 +26,12 @@
 #			'ml' Tab-completion proposes is checked against the
 #			ground truth fetched from 'module help' (see
 #			007-module_help.exp) and 'module config' (see
-#			008-module_config.exp) -- neither less nor more
+#			008-module_config.exp) -- neither less nor more, and
+#			completion candidate words built from untrusted text
+#			(module names read off disk, LOADEDMODULES,
+#			MODULEPATH) cannot reach a shell expansion step -- a
+#			candidate embedding shell code must be listed as an
+#			inert string, never executed
 #		}C%
 #
 ##############################################################################
@@ -480,6 +485,75 @@ completion_bash_close
 unsetenv_loaded_module
 
 
+#
+# candidate words built from untrusted text cannot reach a shell expansion
+# step (see security fix for the bash-completion command injection)
+#
+
+set marker "$env(TESTSUITEDIR)/completion-injection-marker"
+file delete -force $marker
+
+# module/spider avail: malicious module name read off disk
+
+# the marker path is passed through as an env var rather than embedded
+# directly in the filename, since a filename cannot itself contain '/'
+setenv_var INJMARKER $marker
+set evilname {evil$(touch>$INJMARKER)}
+file mkdir $injectdir
+set fd [open "$injectdir/$evilname" w]
+puts $fd {#%Module1.0}
+close $fd
+
+setenv_path_var MODULEPATH $injectdir
+
+completion_bash_start
+
+set got [completion_bash_list {module load }]
+completion_assert_no_exec $marker
+completion_assert_contains $got [list $evilname]
+
+completion_bash_close
+unsetenv_path_var MODULEPATH
+unsetenv_var INJMARKER
+file delete -force $injectdir
+
+# LOADEDMODULES: malicious already-loaded module name
+
+set evilmod [string map [list MARKERPATH $marker] {python$(touch>MARKERPATH)}]
+# _LMFILES_ is set consistently alongside LOADEDMODULES (matching entry
+# count) purely to avoid an unrelated "inconsistent state" warning; its
+# content is not used by the completion script
+setenv_loaded_module [list gcc $evilmod] [list /fake/gcc /fake/evilmod]
+
+completion_bash_start
+
+set got [completion_bash_list {module unload }]
+completion_assert_no_exec $marker
+completion_assert_contains $got [list $evilmod]
+
+completion_bash_close
+unsetenv_loaded_module
+
+# MODULEPATH: malicious path entry (module unuse)
+
+set evilpath [string map [list MARKERPATH $marker] {/opt$(touch>MARKERPATH)}]
+setenv_path_var MODULEPATH /tmp $evilpath
+
+completion_bash_start
+
+## both entries share a leading '/', so type it explicitly: otherwise
+## readline auto-inserts that common prefix on the first Tab and the
+## double-Tab below would only ring the bell instead of listing
+set got [completion_bash_list {module unuse /}]
+completion_assert_no_exec $marker
+completion_assert_contains $got [list $evilpath]
+
+completion_bash_close
+unsetenv_path_var MODULEPATH
+
+file delete -force $marker
+
+
 #
 #  Cleanup
 #