Skip to content

Commit 25c32ec

Browse files
committed
ts: add zsh/fish/tcsh non-regression cases for the completion injection fix
The bash fix in a previous commit only applied to bash: the unquoted- word expansion compgen -W performs on its candidate string is specific to that one builtin. init/zsh-functions/_module.in hands candidates to 'compadd -a <array>', init/fish_completion to 'complete -a "(...)"' (newline-split command output), and init/tcsh_completion.in to a plain backtick command's word list -- in all three, each element becomes a literal candidate string with no further shell expansion, so none of them were ever vulnerable to this bug class. Add the same three injection-safety cases (a malicious module name, LOADEDMODULES entry, and MODULEPATH entry, each embedding shell code) to 031-zsh.exp/041-fish.exp/051-tcsh.exp, to guard against a future regression rather than a known vulnerability. Each shell has its own candidate-display conventions, which change what the literal "contains" check needs to look for: zsh backslash-escapes special characters before display/insertion, fish single-quotes a candidate containing a special character on single-candidate inline completion and (on a multi-candidate pager listing) strips a "(...)" suffix as if it were a candidate's own description, colliding with the embedded parentheses in the crafted candidate itself. fish has no MODULEPATH case: 'unuse' does not list modulepaths there at all. None of this affects the actual security check (completion_assert_no_exec, confirming that the crafted candidate's 'touch' side effect never ran), which needs no such per-shell handling. Also widens the completion timeout for the new section in 051-tcsh.exp: each case there spawns a cold tcsh session with nothing having already warmed up the pty/subprocess pipeline before listing candidates, which occasionally ran past the default 10 second bound under a loaded machine. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
1 parent 1851a13 commit 25c32ec

4 files changed

Lines changed: 249 additions & 0 deletions

File tree

.hunspell.en.dic

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ autotools
263263
availabilities
264264
avx
265265
ba
266+
backtick
266267
backticks
267268
badcommand
268269
baf
@@ -285,6 +286,7 @@ boolvariantname
285286
boolvr
286287
bourne
287288
bugfix
289+
builtin
288290
cachebuild
289291
cacheclear
290292
cachefile
@@ -341,6 +343,7 @@ comgen
341343
commandexp
342344
commandname
343345
compA
346+
compadd
344347
compB
345348
compat
346349
compdef
@@ -842,6 +845,7 @@ subdir
842845
subdirectories
843846
subdirectory
844847
submodule
848+
subprocess
845849
subprojects
846850
subshell
847851
substring

testsuite/completion.00-init/031-zsh.exp

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@
3838
# @comp_*_opts@ flags into the candidate set
3939
# regardless of prefix, so a bare trailing-space
4040
# listing here only ever shows one set at a time
41+
#
42+
# Also checks, like 021-bash.exp, that a candidate word
43+
# built from untrusted text (a module name,
44+
# LOADEDMODULES, MODULEPATH) cannot reach a shell
45+
# expansion step: init/zsh-functions/_module.in only ever
46+
# hands such text to 'compadd -a <array>', where each
47+
# array element is used as a literal candidate string, so
48+
# this is expected to already be safe -- these cases guard
49+
# against a regression, not a known vulnerability
4150
# }C%
4251
#
4352
##############################################################################
@@ -393,6 +402,79 @@ completion_zsh_close
393402
unsetenv_loaded_module
394403

395404

405+
#
406+
# candidate words built from untrusted text cannot reach a shell expansion
407+
# step (see this file's header, and the security fix for the bash-completion
408+
# command injection this guards against a regression of)
409+
#
410+
411+
set marker "$env(TESTSUITEDIR)/completion-injection-marker"
412+
file delete -force $marker
413+
414+
# module avail: malicious module name read off disk
415+
416+
# the marker path is passed through as an env var rather than embedded
417+
# directly in the filename, since a filename cannot itself contain '/'
418+
setenv_var INJMARKER $marker
419+
set evilname {evil$(touch>$INJMARKER)}
420+
file mkdir $injectdir
421+
set fd [open "$injectdir/$evilname" w]
422+
puts $fd {#%Module1.0}
423+
close $fd
424+
425+
setenv_path_var MODULEPATH $injectdir
426+
427+
completion_zsh_start
428+
429+
## _module_avail_mods hands this candidate to 'compadd', which backslash-
430+
## escapes every shell metacharacter in it before display/insertion (the
431+
## same protection this file's header notes _module.in relies on instead of
432+
## bash's compgen -W): the listing shows 'evil\$\(touch\>\$INJMARKER\)', not
433+
## the raw string
434+
set got [completion_zsh_list {module load }]
435+
completion_assert_no_exec $marker
436+
completion_assert_contains $got [list [regsub -all {[$()>]} $evilname {\\&}]]
437+
438+
completion_zsh_close
439+
unsetenv_path_var MODULEPATH
440+
unsetenv_var INJMARKER
441+
file delete -force $injectdir
442+
443+
# LOADEDMODULES: malicious already-loaded module name
444+
445+
set evilmod [string map [list MARKERPATH $marker] {python$(touch>MARKERPATH)}]
446+
setenv_loaded_module [list gcc $evilmod] [list /fake/gcc /fake/evilmod]
447+
448+
completion_zsh_start
449+
450+
set got [completion_zsh_list {module unload }]
451+
completion_assert_no_exec $marker
452+
completion_assert_contains $got [list $evilmod]
453+
454+
completion_zsh_close
455+
unsetenv_loaded_module
456+
457+
# MODULEPATH: malicious path entry (module unuse)
458+
459+
set evilpath [string map [list MARKERPATH $marker] {/opt$(touch>MARKERPATH)}]
460+
setenv_path_var MODULEPATH /tmp $evilpath
461+
462+
completion_zsh_start
463+
464+
## both entries share a leading '/', so type it explicitly: otherwise zle
465+
## auto-inserts that common prefix on the first Tab and the double-Tab below
466+
## would only ring the bell instead of listing (same trap as the "bar"/"ba"
467+
## cases earlier in this file)
468+
set got [completion_zsh_list {module unuse /}]
469+
completion_assert_no_exec $marker
470+
completion_assert_contains $got [list $evilpath]
471+
472+
completion_zsh_close
473+
unsetenv_path_var MODULEPATH
474+
475+
file delete -force $marker
476+
477+
396478
#
397479
# Cleanup
398480
#

testsuite/completion.00-init/041-fish.exp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@
6666
# the word being completed starts with '-' -- so a bare
6767
# trailing-space listing only ever shows one set, same
6868
# as 031-zsh.exp
69+
#
70+
# Also checks, like 021-bash.exp/031-zsh.exp, that a
71+
# candidate word built from untrusted text (a module
72+
# name, LOADEDMODULES) cannot reach a shell expansion
73+
# step: init/fish_completion only ever hands such text to
74+
# 'complete -a "(...)"', where the parenthesized command's
75+
# output is split into literal candidate lines, so this is
76+
# expected to already be safe -- these cases guard against
77+
# a regression, not a known vulnerability. No MODULEPATH
78+
# case here: 'unuse' does not list modulepaths at all in
79+
# fish (see this file's header)
6980
# }C%
7081
#
7182
##############################################################################
@@ -371,6 +382,69 @@ completion_fish_close
371382
unsetenv_loaded_module
372383

373384

385+
#
386+
# candidate words built from untrusted text cannot reach a shell expansion
387+
# step (see this file's header, and the security fix for the bash-completion
388+
# command injection this guards against a regression of)
389+
#
390+
391+
set marker "$env(TESTSUITEDIR)/completion-injection-marker"
392+
file delete -force $marker
393+
394+
# module avail: malicious module name read off disk
395+
396+
# the marker path is passed through as an env var rather than embedded
397+
# directly in the filename, since a filename cannot itself contain '/'
398+
setenv_var INJMARKER $marker
399+
set evilname {evil$(touch>$INJMARKER)}
400+
file mkdir $injectdir
401+
set fd [open "$injectdir/$evilname" w]
402+
puts $fd {#%Module1.0}
403+
close $fd
404+
405+
setenv_path_var MODULEPATH $injectdir
406+
407+
completion_fish_start
408+
409+
## this fixture has only one module, so a single Tab completes it inline
410+
## rather than showing a pager listing (see completion_fish_list's own
411+
## single-candidate fallback) -- and fish, like zsh, protects a candidate
412+
## containing shell metacharacters before inserting it: fish 4 wraps it in
413+
## single quotes whereas fish 3 backslash-escapes each metacharacter
414+
set got [completion_fish_list {module load }]
415+
completion_assert_no_exec $marker
416+
completion_assert_any $got [list "'$evilname'"\
417+
[regsub -all {[$()>]} $evilname {\\&}]]
418+
419+
completion_fish_close
420+
unsetenv_path_var MODULEPATH
421+
unsetenv_var INJMARKER
422+
file delete -force $injectdir
423+
424+
# LOADEDMODULES: malicious already-loaded module name
425+
426+
set evilmod [string map [list MARKERPATH $marker] {python$(touch>MARKERPATH)}]
427+
setenv_loaded_module [list gcc $evilmod] [list /fake/gcc /fake/evilmod]
428+
429+
completion_fish_start
430+
431+
## two candidates here (gcc and evilmod) means a real pager listing, which
432+
## completion_fish_list runs through completion_fish_strip_descriptions to
433+
## drop each candidate's "(description)" suffix -- the injected
434+
## '(touch>...)' substring is indistinguishable from that convention and
435+
## gets stripped the same way, so the surviving candidate is the text up to
436+
## its own first '(', not the full raw string (still proven never executed
437+
## by completion_assert_no_exec above)
438+
set got [completion_fish_list {module unload }]
439+
completion_assert_no_exec $marker
440+
completion_assert_contains $got [list [completion_fish_strip_descriptions $evilmod]]
441+
442+
completion_fish_close
443+
unsetenv_loaded_module
444+
445+
file delete -force $marker
446+
447+
374448
#
375449
# Cleanup
376450
#

testsuite/completion.00-init/051-tcsh.exp

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,18 @@
7878
# worked around here, since 'help' otherwise dispatches
7979
# exactly like 'show'/'display'/'test'/... which all
8080
# already carry that macro)
81+
#
82+
# Also checks, like 021-bash.exp/031-zsh.exp, that a
83+
# candidate word built from untrusted text (a module
84+
# name, LOADEDMODULES, MODULEPATH) cannot reach a shell
85+
# expansion step: init/tcsh_completion.in only ever hands
86+
# such text to a plain 'n/.../`...`/' backtick rule, where
87+
# each output line becomes a literal candidate string, so
88+
# this is expected to already be safe -- these cases guard
89+
# against a regression, not a known vulnerability. The
90+
# MODULEPATH case uses a bare 'module unuse ' (no typed
91+
# prefix), same restriction as this file's own 'unuse'
92+
# section above
8193
# }C%
8294
#
8395
##############################################################################
@@ -455,6 +467,83 @@ completion_tcsh_close
455467
unsetenv_loaded_module
456468

457469

470+
#
471+
# candidate words built from untrusted text cannot reach a shell expansion
472+
# step (see this file's header, and the security fix for the bash-completion
473+
# command injection this guards against a regression of)
474+
#
475+
476+
# each case below spawns a fresh tcsh session and, unlike every other
477+
# section in this file, its Ctrl-D listing goes through _module_loaded/
478+
# _module_avail/_module_modulepath cold (no completion call already primed
479+
# the pty/subprocess pipeline): comfortably inside the default 10s
480+
# 'timeout' most of the time, but occasionally not under a loaded machine,
481+
# so it is raised for just this section rather than risking a false
482+
# UNRESOLVED
483+
set old_timeout $timeout
484+
set timeout 30
485+
486+
set marker "$env(TESTSUITEDIR)/completion-injection-marker"
487+
file delete -force $marker
488+
489+
# module avail: malicious module name read off disk
490+
491+
# the marker path is passed through as an env var rather than embedded
492+
# directly in the filename, since a filename cannot itself contain '/'
493+
setenv_var INJMARKER $marker
494+
set evilname {evil$(touch>$INJMARKER)}
495+
file mkdir $injectdir
496+
set fd [open "$injectdir/$evilname" w]
497+
puts $fd {#%Module1.0}
498+
close $fd
499+
500+
setenv_path_var MODULEPATH $injectdir
501+
502+
completion_tcsh_start
503+
504+
set got [completion_tcsh_list {module load }]
505+
completion_assert_no_exec $marker
506+
completion_assert_contains $got [list $evilname]
507+
508+
completion_tcsh_close
509+
unsetenv_path_var MODULEPATH
510+
unsetenv_var INJMARKER
511+
file delete -force $injectdir
512+
513+
# LOADEDMODULES: malicious already-loaded module name
514+
515+
set evilmod [string map [list MARKERPATH $marker] {python$(touch>MARKERPATH)}]
516+
setenv_loaded_module [list gcc $evilmod] [list /fake/gcc /fake/evilmod]
517+
518+
completion_tcsh_start
519+
520+
set got [completion_tcsh_list {module unload }]
521+
completion_assert_no_exec $marker
522+
completion_assert_contains $got [list $evilmod]
523+
524+
completion_tcsh_close
525+
unsetenv_loaded_module
526+
527+
# MODULEPATH: malicious path entry (module unuse); like this file's own
528+
# 'unuse' section above, only a bare, empty word reaches the modulepath
529+
# listing at all (see this file's header)
530+
531+
set evilpath [string map [list MARKERPATH $marker] {/opt$(touch>MARKERPATH)}]
532+
setenv_path_var MODULEPATH /tmp $evilpath
533+
534+
completion_tcsh_start
535+
536+
set got [completion_tcsh_list {module unuse }]
537+
completion_assert_no_exec $marker
538+
completion_assert_contains $got [list $evilpath]
539+
540+
completion_tcsh_close
541+
unsetenv_path_var MODULEPATH
542+
543+
set timeout $old_timeout
544+
file delete -force $marker
545+
546+
458547
#
459548
# Cleanup
460549
#

0 commit comments

Comments
 (0)