Skip to content

Commit 6c0631d

Browse files
mrcawoodmrcawood-dell
mrcawood
authored and
mrcawood-dell
committed
sh: complete bash tab-completion command-injection fix
Finish converting untrusted completion candidates from compgen -W to _module_comgen_words, building on Robert McLay's injection branch work. Cover _module_long_arg_list, MODULEPATH/unuse, _ml fallbacks, and opts/cmds. Add rt/bash_completion regression check and 9.3.1 release note. Found by AISLE in partnership with Red Hat.
1 parent 04c8deb commit 6c0631d

6 files changed

Lines changed: 191 additions & 42 deletions

File tree

‎docs/source/025_new.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ New Features in Lmod
88
and :ref:`lua_modulefile_functions-label`. Rationale:
99
`Lmod issue #849 <https://github.com/TACC/Lmod/issues/849>`__.
1010

11+
**Bash tab completion treats module names as literal words**
12+
(Lmod 9.3.4+) Bash completion for ``module`` and ``ml`` no longer
13+
expands command substitution in module names, collection names, or
14+
MODULEPATH entries. Found by AISLE in partnership with Red Hat.
15+
1116
**Support for Package Managers to have their own SitePackage.lua files**
1217
(Lmod 9.3+) Lmod now support the env. var. LMOD_SITEPACKAGE_PREPEND.
1318
This is a colon separated list of absolute paths to Lua files.

‎init/lmod_bash_completions‎

Lines changed: 29 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ _module_comgen_words() {
44
# split candidate list on IFS without triggering shell expansion (command
55
# substitution, arithmetic, ...): candidate words may come from module
66
# names read off disk, which must never be treated as executable code
7-
IFS=$' \t\n' read -r -d '' -a words <<<"$1"
7+
IFS=$' \t\n' read -r -d '' -a words <<<"$1" || true
88
for val in "${words[@]}"; do
99
case "$val" in
1010
"$2"*) COMPREPLY[${#COMPREPLY[@]}]="$val" ;;
@@ -112,17 +112,17 @@ _module_not_yet_loaded() {
112112
_module_long_arg_list() {
113113
local cur="${1}" i
114114
if [[ ${COMP_WORDS[COMP_CWORD-2]} == sw* ]]; then
115-
COMPREPLY=( $(compgen -W "$(_module_not_yet_loaded)" -- "${cur}") )
115+
_module_comgen_words "$(_module_not_yet_loaded)" "${cur}"
116116
return
117117
fi
118118
for ((i = COMP_CWORD - 1; i > 0; i--)); do
119119
case ${COMP_WORDS[${i}]} in
120120
add|load)
121-
COMPREPLY=( $(compgen -W "$(_module_not_yet_loaded)" -- "${cur}") )
121+
_module_comgen_words "$(_module_not_yet_loaded)" "${cur}"
122122
break
123123
;;
124124
rm|remove|unload|switch|swap)
125-
COMPREPLY=( $(compgen -W "$(_module_loaded_modules)" -- "${cur}") )
125+
_module_comgen_words "$(_module_loaded_modules)" "${cur}"
126126
break
127127
;;
128128
esac
@@ -147,38 +147,31 @@ _module() {
147147

148148
case "${prev}" in
149149
add|load|try-load)
150-
#COMPREPLY=( $(compgen -W "$(_module_not_yet_loaded)" -- "${cur}") )
151-
_module_comgen_words "$(_module_not_yet_loaded)" "$cur"
150+
_module_comgen_words "$(_module_not_yet_loaded)" "${cur}"
152151
;;
153152
rm|remove|unload|switch|swap)
154-
#COMPREPLY=( $(compgen -W "$(_module_loaded_modules)" -- "${cur}") )
155-
_module_comgen_words "$(_module_loaded_modules)" "$cur"
153+
_module_comgen_words "$(_module_loaded_modules)" "${cur}"
156154
;;
157155
restore)
158-
#COMPREPLY=( $(compgen -W "$(_module_savelist)" -- "${cur}") )
159-
_module_comgen_words "$(_module_savelist)" "$cur"
156+
_module_comgen_words "$(_module_savelist)" "${cur}"
160157
;;
161158
spider)
162-
#COMPREPLY=( $(compgen -W "$(_module_spider)" -- "${cur}") )
163-
_module_comgen_words "$(_module_spider)" "$cur"
159+
_module_comgen_words "$(_module_spider)" "${cur}"
164160
;;
165161
unuse)
166-
COMPREPLY=( $(IFS=: compgen -W "${MODULEPATH}" -- "${cur}") )
162+
_module_comgen_words "${MODULEPATH//:/ }" "${cur}"
167163
;;
168164
use|*-a*)
169165
_module_dir "${cur}"
170166
;;
171167
help|show|whatis)
172-
#COMPREPLY=( $(compgen -W "$(_module_avail)" -- "${cur}") )
173-
_module_comgen_words "$(_module_avail)" "$cur"
168+
_module_comgen_words "$(_module_avail)" "${cur}"
174169
;;
175170
describe|mcc)
176-
#COMPREPLY=( $(compgen -W "$(_module_mcc)" -- "${cur}") )
177-
_module_comgen_words "$(_module_mcc)" "$cur"
171+
_module_comgen_words "$(_module_mcc)" "${cur}"
178172
;;
179173
disable)
180-
#COMPREPLY=( $(compgen -W "$(_module_mcc)" -- "${cur}") )
181-
_module_comgen_words "$(_module_mcc)" "$cur"
174+
_module_comgen_words "$(_module_mcc)" "${cur}"
182175
;;
183176
*)
184177
if [ ${COMP_CWORD} -gt 2 ]; then
@@ -193,10 +186,10 @@ _module() {
193186
COMPREPLY='swap'
194187
;;
195188
-*)
196-
COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
189+
_module_comgen_words "${opts}" "${cur}"
197190
;;
198191
*)
199-
COMPREPLY=( $(compgen -W "${cmds}" -- "${cur}") )
192+
_module_comgen_words "${cmds}" "${cur}"
200193
;;
201194
esac
202195
fi
@@ -223,42 +216,36 @@ _ml() {
223216

224217
case "${prev}" in
225218
rm|remove|unload|switch|swap)
226-
#COMPREPLY=( $(compgen -W "$(_module_loaded_modules)" -- "${cur}") )
227-
_module_comgen_words "$(_module_loaded_modules)" "$cur"
219+
_module_comgen_words "$(_module_loaded_modules)" "${cur}"
228220
;;
229221
restore)
230-
#COMPREPLY=( $(compgen -W "$(_module_savelist)" -- "${cur}") )
231-
_module_comgen_words "$(_module_savelist)" "$cur"
222+
_module_comgen_words "$(_module_savelist)" "${cur}"
232223
;;
233224
spider)
234-
#COMPREPLY=( $(compgen -W "$(_module_spider)" -- "${cur}") )
235-
_module_comgen_words "$(_module_spider)" "$cur"
225+
_module_comgen_words "$(_module_spider)" "${cur}"
236226
;;
237227
unuse)
238-
COMPREPLY=( $(IFS=: compgen -W "${MODULEPATH}" -- "${cur}") )
228+
_module_comgen_words "${MODULEPATH//:/ }" "${cur}"
239229
;;
240230
use|*-a*)
241231
_module_dir "${cur}"
242232
;;
243233
help|show|whatis)
244-
#COMPREPLY=( $(compgen -W "$(_module_avail)" -- "${cur}") )
245-
_module_comgen_words "$(_module_avail)" "$cur"
234+
_module_comgen_words "$(_module_avail)" "${cur}"
246235
;;
247236
describe|mcc)
248-
#COMPREPLY=( $(compgen -W "$(_module_mcc)" -- "${cur}") )
249-
_module_comgen_words "$(_module_mcc)" "$cur"
237+
_module_comgen_words "$(_module_mcc)" "${cur}"
250238
;;
251239
disable)
252-
#COMPREPLY=( $(compgen -W "$(_module_mcc)" -- "${cur}") )
253-
_module_comgen_words "$(_module_mcc)" "$cur"
240+
_module_comgen_words "$(_module_mcc)" "${cur}"
254241
;;
255242
*)
256243
case "${cur}" in
257244
-*)
258245
if [ ${COMP_CWORD} -eq 1 ]; then
259-
COMPREPLY=( $(compgen -W "${opts} $(_module_loaded_modules_negated)" -- "${cur}") )
246+
_module_comgen_words "${opts} $(_module_loaded_modules_negated)" "${cur}"
260247
else
261-
COMPREPLY=( $(compgen -W " $(_module_loaded_modules_negated)" -- "${cur}") )
248+
_module_comgen_words "$(_module_loaded_modules_negated)" "${cur}"
262249
fi
263250
;;
264251
*)
@@ -271,34 +258,34 @@ _ml() {
271258
COMPREPLY='swap'
272259
;;
273260
*)
274-
COMPREPLY=( $(compgen -W "${cmds} $(_module_avail)" -- "${cur}") )
261+
_module_comgen_words "${cmds} $(_module_avail)" "${cur}"
275262
;;
276263
esac
277264
else
278265
if [[ ${COMP_WORDS[COMP_CWORD-2]} == sw* ]]; then
279-
COMPREPLY=( $(compgen -W "$(_module_not_yet_loaded)" -- "${cur}") )
266+
_module_comgen_words "$(_module_not_yet_loaded)" "${cur}"
280267
else
281268
for ((i = COMP_CWORD - 1; i > 0; i--)); do
282269
case ${COMP_WORDS[$i]} in
283270
show|whatis)
284-
COMPREPLY=( $(compgen -W "$(_module_avail)" -- "${cur}") )
271+
_module_comgen_words "$(_module_avail)" "${cur}"
285272
found=1
286273
break
287274
;;
288275
rm|remove|unload)
289-
COMPREPLY=( $(compgen -W "$(_module_loaded_modules)" -- "${cur}") )
276+
_module_comgen_words "$(_module_loaded_modules)" "${cur}"
290277
found=1
291278
break
292279
;;
293280
spider)
294-
COMPREPLY=( $(compgen -W "$(_module_spider)" -- "${cur}") )
281+
_module_comgen_words "$(_module_spider)" "${cur}"
295282
found=1
296283
break
297284
;;
298285
esac
299286
done
300287
if [ -z "${found}" ]; then
301-
COMPREPLY=( $(compgen -W "$(_module_avail)" -- "${cur}") )
288+
_module_comgen_words "$(_module_avail)" "${cur}"
302289
fi
303290
fi
304291
fi

‎rt/bash_completion/bash_completion.tdesc‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
-- -*- lua -*-
2+
local testName = "bash_completion"
3+
4+
testdescript = {
5+
owner = "rtm",
6+
product = "modules",
7+
description = [[
8+
Bash tab completion must not expand module names as shell code
9+
]],
10+
keywords = {testName },
11+
12+
active = 1,
13+
testName = testName,
14+
job_submit_method = "INTERACTIVE",
15+
16+
runScript = [[
17+
18+
. $(projectDir)/rt/common_funcs.sh
19+
20+
unsetMT
21+
initStdEnvVars
22+
23+
remove_generated_lmod_files
24+
25+
runLmod --version # 1
26+
runBase bash $(testDir)/check_compgen.sh $(projectDir)/init/lmod_bash_completions # 2
27+
28+
HOME=$ORIG_HOME
29+
cat _stdout.[0-9][0-9][0-9] > _stdout.orig
30+
joinBase64Results -bash _stdout.orig _stdout.new
31+
cleanUp _stdout.new out.txt
32+
33+
cat _stderr.[0-9][0-9][0-9] > _stderr.orig
34+
cleanUp _stderr.orig err.txt
35+
36+
rm -f results.csv
37+
wrapperDiff --csv results.csv $(testDir)/out.txt out.txt
38+
wrapperDiff --csv results.csv $(testDir)/err.txt err.txt
39+
testFinish -r $(resultFn) -t $(runtimeFn) results.csv
40+
]],
41+
42+
43+
blessScript = [[
44+
# perform what is needed
45+
]],
46+
47+
tests = {
48+
{ id='t1'},
49+
},
50+
51+
}

‎rt/bash_completion/check_compgen.sh‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
#!/usr/bin/env bash
2+
# Programmable-completion check for init/lmod_bash_completions.
3+
# Candidate strings that look like command substitution must stay literal.
4+
# This drives the completion functions directly. It does not press Tab.
5+
6+
set -u
7+
8+
src=${1:?usage: check_compgen.sh /path/to/lmod_bash_completions}
9+
# shellcheck disable=SC1090
10+
. "$src"
11+
12+
# Stub discovery so this test does not run Lmod or need @PKG@ substituted.
13+
_module_avail() {
14+
printf '%s\n' 'safe' '$(:)' 'other'
15+
}
16+
_module_loaded_modules() { :; }
17+
_module_not_yet_loaded() { _module_avail; }
18+
_module_spider() { :; }
19+
_module_savelist() { :; }
20+
_module_mcc() { :; }
21+
_module_loaded_modules_negated() { :; }
22+
23+
has_word() {
24+
local want=$1
25+
shift
26+
local w
27+
for w in "$@"; do
28+
if [[ "$w" == "$want" ]]; then
29+
return 0
30+
fi
31+
done
32+
return 1
33+
}
34+
35+
report() {
36+
local label=$1
37+
shift
38+
if has_word '$(:)' "$@"; then
39+
echo "${label}_literal=yes"
40+
else
41+
echo "${label}_literal=no"
42+
fi
43+
if has_word 'safe' "$@"; then
44+
echo "${label}_safe=yes"
45+
else
46+
echo "${label}_safe=no"
47+
fi
48+
}
49+
50+
COMPREPLY=()
51+
_module_comgen_words 'safe $(:) other' ''
52+
report helper "${COMPREPLY[@]}"
53+
54+
COMPREPLY=()
55+
_module_comgen_words 'safe $(printf %s INJECTED) other' ''
56+
if has_word 'INJECTED' "${COMPREPLY[@]}"; then
57+
echo helper_expanded=yes
58+
else
59+
echo helper_expanded=no
60+
fi
61+
62+
COMPREPLY=()
63+
_module_comgen_words 'safe $(:) other' 'saf'
64+
if [[ ${#COMPREPLY[@]} -eq 1 && ${COMPREPLY[0]} == safe ]]; then
65+
echo prefix_safe=yes
66+
else
67+
echo prefix_safe=no
68+
fi
69+
70+
COMPREPLY=()
71+
COMP_WORDS=(module load)
72+
COMP_CWORD=2
73+
_module module '' load
74+
report load "${COMPREPLY[@]}"
75+
76+
COMPREPLY=()
77+
COMP_WORDS=(ml)
78+
COMP_CWORD=1
79+
_ml ml '' ml
80+
report ml "${COMPREPLY[@]}"

‎rt/bash_completion/err.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
===========================
2+
step 1
3+
lua ProjectDIR/src/lmod.in.lua shell --regression_testing --version
4+
===========================
5+
Modules based on Lua: Version 9.3 2026-07-28 08:08 -06:00
6+
https://lmod.readthedocs.io
7+
===========================
8+
step 2
9+
bash ProjectDIR/rt/bash_completion/check_compgen.sh ProjectDIR/init/lmod_bash_completions
10+
===========================

‎rt/bash_completion/out.txt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
===========================
2+
step 1
3+
lua ProjectDIR/src/lmod.in.lua shell --regression_testing --version
4+
===========================
5+
===========================
6+
step 2
7+
bash ProjectDIR/rt/bash_completion/check_compgen.sh ProjectDIR/init/lmod_bash_completions
8+
===========================
9+
helper_literal=yes
10+
helper_safe=yes
11+
helper_expanded=no
12+
prefix_safe=yes
13+
load_literal=yes
14+
load_safe=yes
15+
ml_literal=yes
16+
ml_safe=yes

0 commit comments

Comments
 (0)