Skip to content

Commit bd44f57

Browse files
xdelaruellexdelaruelle
committed
ts: add bash non-regression cases for the completion injection fix
Adds injection-safety cases to the completion DejaGnu tool (testsuite/completion.00-init/021-bash.exp) covering the fix in the previous commit: a crafted module name, LOADEDMODULES entry, and MODULEPATH entry embedding shell code must be listed as an inert candidate string and never executed, verified by checking a marker file the payload would touch if it ran (completion_assert_no_exec, new in 006-procs.exp). Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
1 parent 53309ea commit bd44f57

5 files changed

Lines changed: 105 additions & 7 deletions

File tree

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@
6161
/testsuite/completion-use-fixture
6262
/testsuite/completion-home
6363
/testsuite/cookbook-sandbox
64+
/testsuite/completion-inject-fixture
65+
/testsuite/completion-injection-marker
6466
/doc/build
6567
/doc/source/version.py
6668
/doc/demo/*/*.svg

‎doc/source/devel/testsuite.rst‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ DejaGnu tools:
4646
Drives a real, interactive shell process (via Expect ``spawn``/``send``/
4747
``expect``, not just a captured non-interactive run like the other three
4848
tools) to press Tab against the built shell completion script and check
49-
that the resulting candidate list holds the expected module names and
50-
option flags. Driven by the :file:`completion.00-init` directory;
49+
the resulting candidate list -- both that expected module names/option
50+
flags show up, and that a candidate word built from untrusted text (a
51+
module name, ``LOADEDMODULES``, ``MODULEPATH``, ...) can never reach a
52+
shell expansion step. Driven by the :file:`completion.00-init` directory;
5153
currently covers bash, zsh, fish and tcsh, see `completion.00-init
5254
layout`_.
5355

@@ -181,9 +183,9 @@ shell-specific in a way none of the other three tools are:
181183
:file:`init/zsh-functions/_module`, :file:`init/fish_completion` and
182184
:file:`init/tcsh_completion` scripts, a clean fixture modulepath), the
183185
shell-agnostic assert procedures (``completion_assert_contains``,
184-
``completion_assert_not_contains``, ``completion_assert_eq``), and the
185-
clean baseline environment/``save_test_env`` checkpoint, exactly as for
186-
the other tools.
186+
``completion_assert_not_contains``, ``completion_assert_eq``,
187+
``completion_assert_no_exec``), and the clean baseline
188+
environment/``save_test_env`` checkpoint, exactly as for the other tools.
187189
- ``0NN-<shell>-procs.exp`` defines one ``completion_<shell>_start`` /
188190
``completion_<shell>_raw`` / ``completion_<shell>_list`` /
189191
``completion_<shell>_inline`` / ``completion_<shell>_close`` set per

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,8 @@ set out [open $tcshcompletion w]
121121
puts -nonewline $out $tcshcompletion_content
122122
close $out
123123

124+
# scratch area for modulefiles created on the fly by completion tests
125+
set injectdir "$env(TESTSUITEDIR)/completion-inject-fixture"
126+
file delete -force $injectdir
127+
124128
# vim:set tabstop=3 shiftwidth=3 expandtab autoindent:

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,20 @@ proc completion_assert_eq {got expected} {
130130
}
131131
}
132132

133+
# assert a marker file was not created (used to prove no shell code
134+
# injected through a candidate word got executed); the test label is built
135+
# from the cmdline last passed to completion_<shell>_list plus the marker
136+
# path
137+
proc completion_assert_no_exec {markerpath} {
138+
global completion_last_cmdline
139+
140+
set label "'$completion_last_cmdline' completion does not execute injected code ($markerpath)"
141+
if {[file exists $markerpath]} {
142+
fail "$label (marker file was created: injected code ran)"
143+
file delete -force $markerpath
144+
} else {
145+
pass $label
146+
}
147+
}
148+
133149
# vim:set tabstop=3 shiftwidth=3 expandtab autoindent:

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

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
# Authors: Xavier Delaruelle, xavier.delaruelle@cea.fr
1111
#
1212
# Description: Testuite testsequence
13-
# Command: avail, load, unload, ml, use, unuse, restore
13+
# Command: avail, load, unload, ml, use, unuse, restore, switch
1414
# Sub-Command:
1515
#
1616
# Comment: %C{
@@ -26,7 +26,12 @@
2626
# 'ml' Tab-completion proposes is checked against the
2727
# ground truth fetched from 'module help' (see
2828
# 007-module_help.exp) and 'module config' (see
29-
# 008-module_config.exp) -- neither less nor more
29+
# 008-module_config.exp) -- neither less nor more, and
30+
# completion candidate words built from untrusted text
31+
# (module names read off disk, LOADEDMODULES,
32+
# MODULEPATH) cannot reach a shell expansion step -- a
33+
# candidate embedding shell code must be listed as an
34+
# inert string, never executed
3035
# }C%
3136
#
3237
##############################################################################
@@ -480,6 +485,75 @@ completion_bash_close
480485
unsetenv_loaded_module
481486

482487

488+
#
489+
# candidate words built from untrusted text cannot reach a shell expansion
490+
# step (see security fix for the bash-completion command injection)
491+
#
492+
493+
set marker "$env(TESTSUITEDIR)/completion-injection-marker"
494+
file delete -force $marker
495+
496+
# module/spider avail: malicious module name read off disk
497+
498+
# the marker path is passed through as an env var rather than embedded
499+
# directly in the filename, since a filename cannot itself contain '/'
500+
setenv_var INJMARKER $marker
501+
set evilname {evil$(touch>$INJMARKER)}
502+
file mkdir $injectdir
503+
set fd [open "$injectdir/$evilname" w]
504+
puts $fd {#%Module1.0}
505+
close $fd
506+
507+
setenv_path_var MODULEPATH $injectdir
508+
509+
completion_bash_start
510+
511+
set got [completion_bash_list {module load }]
512+
completion_assert_no_exec $marker
513+
completion_assert_contains $got [list $evilname]
514+
515+
completion_bash_close
516+
unsetenv_path_var MODULEPATH
517+
unsetenv_var INJMARKER
518+
file delete -force $injectdir
519+
520+
# LOADEDMODULES: malicious already-loaded module name
521+
522+
set evilmod [string map [list MARKERPATH $marker] {python$(touch>MARKERPATH)}]
523+
# _LMFILES_ is set consistently alongside LOADEDMODULES (matching entry
524+
# count) purely to avoid an unrelated "inconsistent state" warning; its
525+
# content is not used by the completion script
526+
setenv_loaded_module [list gcc $evilmod] [list /fake/gcc /fake/evilmod]
527+
528+
completion_bash_start
529+
530+
set got [completion_bash_list {module unload }]
531+
completion_assert_no_exec $marker
532+
completion_assert_contains $got [list $evilmod]
533+
534+
completion_bash_close
535+
unsetenv_loaded_module
536+
537+
# MODULEPATH: malicious path entry (module unuse)
538+
539+
set evilpath [string map [list MARKERPATH $marker] {/opt$(touch>MARKERPATH)}]
540+
setenv_path_var MODULEPATH /tmp $evilpath
541+
542+
completion_bash_start
543+
544+
## both entries share a leading '/', so type it explicitly: otherwise
545+
## readline auto-inserts that common prefix on the first Tab and the
546+
## double-Tab below would only ring the bell instead of listing
547+
set got [completion_bash_list {module unuse /}]
548+
completion_assert_no_exec $marker
549+
completion_assert_contains $got [list $evilpath]
550+
551+
completion_bash_close
552+
unsetenv_path_var MODULEPATH
553+
554+
file delete -force $marker
555+
556+
483557
#
484558
# Cleanup
485559
#

0 commit comments

Comments
 (0)