Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unintended command execution by "_quote_readline_by_ref" #492

Merged
merged 8 commits into from
May 21, 2021
15 changes: 10 additions & 5 deletions bash_completion
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,17 @@ _quote_readline_by_ref()
printf -v $2 %s "${1:1}"
else
printf -v $2 %q "$1"
fi

# If result becomes quoted like this: $'string', re-evaluate in order to
# drop the additional quoting. See also:
# https://www.mail-archive.com/[email protected]/msg01942.html
[[ ${!2} == \$* ]] && eval $2=${!2}
# If result becomes quoted like this: $'string', re-evaluate in order
# to drop the additional quoting. See also:
# https://www.mail-archive.com/[email protected]/msg01942.html
if [[ ${!2} == \$\'*\' ]]; then
local value=${!2:2:-1} # Strip beginning $' and ending '.
value=${value//'%'/%%} # Escape % for printf format.
printf -v value %s "$value" # Decode escape sequences of \....
akinomyoga marked this conversation as resolved.
Show resolved Hide resolved
local "$2" && _upvars -v "$2" "$value"
fi
fi
} # _quote_readline_by_ref()

# This function performs file and directory completion. It's better than
Expand Down
174 changes: 96 additions & 78 deletions test/t/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,88 +195,106 @@ def bash(request) -> pexpect.spawn:
)
)

fixturesdir = os.path.join(testdir, "fixtures")
os.chdir(fixturesdir)

# Start bash
bash = pexpect.spawn(
"%s --norc" % os.environ.get("BASHCOMP_TEST_BASH", "bash"),
maxread=os.environ.get("BASHCOMP_TEST_PEXPECT_MAXREAD", 20000),
logfile=logfile,
cwd=fixturesdir,
env=env,
encoding="utf-8", # TODO? or native or...?
# FIXME: Tests shouldn't depend on dimensions, but it's difficult to
# expect robustly enough for Bash to wrap lines anywhere (e.g. inside
# MAGIC_MARK). Increase window width to reduce wrapping.
dimensions=(24, 160),
# TODO? codec_errors="replace",
)
bash.expect_exact(PS1)

# Load bashrc and bash_completion
assert_bash_exec(bash, "source '%s/config/bashrc'" % testdir)
assert_bash_exec(bash, "source '%s/../bash_completion'" % testdir)

# Use command name from marker if set, or grab from test filename
cmd = None # type: Optional[str]
cmd_found = False
marker = request.node.get_closest_marker("bashcomp")
if marker:
cmd = marker.kwargs.get("cmd")
cmd_found = "cmd" in marker.kwargs
# Run pre-test commands, early so they're usable in skipif
for pre_cmd in marker.kwargs.get("pre_cmds", []):
assert_bash_exec(bash, pre_cmd, want_output=None)
# Process skip and xfail conditions
skipif = marker.kwargs.get("skipif")
if skipif:
try:
assert_bash_exec(bash, skipif, want_output=None)
except AssertionError:
pass
else:
bash.close()
pytest.skip(skipif)
xfail = marker.kwargs.get("xfail")
if xfail:
try:
assert_bash_exec(bash, xfail, want_output=None)
except AssertionError:
pass
else:
pytest.xfail(xfail)
if not cmd_found:
match = re.search(
r"^test_(.+)\.py$", os.path.basename(str(request.fspath))
tmpdir = None
bash = None
try:
marker = request.node.get_closest_marker("bashcomp")

# Set up the current working directory
cwd = None
if marker:
if "cwd" in marker.kwargs and marker.kwargs.get("cwd") is not None:
cwd = marker.kwargs.get("cwd")
elif "temp_cwd" in marker.kwargs and marker.kwargs.get("temp_cwd"):
tmpdir = tempfile.TemporaryDirectory()
cwd = tmpdir.name
if cwd is None:
cwd = os.path.join(testdir, "fixtures")
os.chdir(cwd)

# Start bash
bash = pexpect.spawn(
"%s --norc" % os.environ.get("BASHCOMP_TEST_BASH", "bash"),
maxread=os.environ.get("BASHCOMP_TEST_PEXPECT_MAXREAD", 20000),
logfile=logfile,
cwd=cwd,
env=env,
encoding="utf-8", # TODO? or native or...?
# FIXME: Tests shouldn't depend on dimensions, but it's difficult to
# expect robustly enough for Bash to wrap lines anywhere (e.g. inside
# MAGIC_MARK). Increase window width to reduce wrapping.
dimensions=(24, 160),
# TODO? codec_errors="replace",
)
if match:
cmd = match.group(1)

request.cls.cmd = cmd

if (cmd_found and cmd is None) or is_testable(bash, cmd):
before_env = get_env(bash)
yield bash
# Not exactly sure why, but some errors leave bash in state where
# getting the env here would fail and trash our test output. So
# reset to a good state first (Ctrl+C, expect prompt).
bash.sendintr()
bash.expect_exact(PS1)
diff_env(
before_env,
get_env(bash),
marker.kwargs.get("ignore_env") if marker else "",
)

if marker:
for post_cmd in marker.kwargs.get("post_cmds", []):
assert_bash_exec(bash, post_cmd, want_output=None)
# Load bashrc and bash_completion
assert_bash_exec(bash, "source '%s/config/bashrc'" % testdir)
assert_bash_exec(bash, "source '%s/../bash_completion'" % testdir)

# Use command name from marker if set, or grab from test filename
cmd = None # type: Optional[str]
cmd_found = False
if marker:
cmd = marker.kwargs.get("cmd")
cmd_found = "cmd" in marker.kwargs
# Run pre-test commands, early so they're usable in skipif
for pre_cmd in marker.kwargs.get("pre_cmds", []):
assert_bash_exec(bash, pre_cmd, want_output=None)
# Process skip and xfail conditions
skipif = marker.kwargs.get("skipif")
if skipif:
try:
assert_bash_exec(bash, skipif, want_output=None)
except AssertionError:
pass
else:
bash.close()
bash = None
pytest.skip(skipif)
xfail = marker.kwargs.get("xfail")
if xfail:
try:
assert_bash_exec(bash, xfail, want_output=None)
except AssertionError:
pass
else:
pytest.xfail(xfail)
if not cmd_found:
match = re.search(
r"^test_(.+)\.py$", os.path.basename(str(request.fspath))
)
if match:
cmd = match.group(1)

request.cls.cmd = cmd

if (cmd_found and cmd is None) or is_testable(bash, cmd):
before_env = get_env(bash)
yield bash
# Not exactly sure why, but some errors leave bash in state where
# getting the env here would fail and trash our test output. So
# reset to a good state first (Ctrl+C, expect prompt).
bash.sendintr()
bash.expect_exact(PS1)
diff_env(
before_env,
get_env(bash),
marker.kwargs.get("ignore_env") if marker else "",
)

if marker:
for post_cmd in marker.kwargs.get("post_cmds", []):
assert_bash_exec(bash, post_cmd, want_output=None)

# Clean up
bash.close()
if logfile and logfile != sys.stdout:
logfile.close()
finally:
# Clean up
if bash:
bash.close()
if tmpdir:
tmpdir.cleanup()
if logfile and logfile != sys.stdout:
logfile.close()


def is_testable(bash: pexpect.spawn, cmd: Optional[str]) -> bool:
Expand Down
79 changes: 78 additions & 1 deletion test/t/unit/test_unit_quote_readline.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import os

import pytest

from conftest import assert_bash_exec


@pytest.mark.bashcomp(cmd=None)
@pytest.mark.bashcomp(cmd=None, temp_cwd=True)
class TestUnitQuoteReadline:
def test_exec(self, bash):
assert_bash_exec(bash, "quote_readline '' >/dev/null")
Expand All @@ -13,3 +15,78 @@ def test_env_non_pollution(self, bash):
assert_bash_exec(
bash, "foo() { quote_readline meh >/dev/null; }; foo; unset foo"
)

def test_github_issue_189_1(self, bash):
"""Test error messages on a certain command line

Reported at https://github.com/scop/bash-completion/issues/189

Syntax error messages should not be shown by completion on the
following line:

$ ls -- '${[TAB]
$ rm -- '${[TAB]

"""
assert_bash_exec(bash, "quote_readline $'\\'${' >/dev/null")

def test_github_issue_492_1(self, bash):
"""Test unintended code execution on a certain command line

Reported at https://github.com/scop/bash-completion/pull/492

Arbitrary commands could be unintendedly executed by
_quote_readline_by_ref. In the following example, the command
"touch 1.txt" would be unintendedly created before the fix. The file
"1.txt" should not be created by completion on the following line:

$ echo '$(touch file.txt)[TAB]

"""
assert_bash_exec(
bash, "quote_readline $'\\'$(touch 1.txt)' >/dev/null"
)
assert not os.path.exists("./1.txt")

def test_github_issue_492_2(self, bash):
"""Test the file clear by unintended redirection on a certain command line

Reported at https://github.com/scop/bash-completion/pull/492

The file "1.0" should not be created by completion on the following
line:

$ awk '$1 > 1.0[TAB]

"""
assert_bash_exec(bash, "quote_readline $'\\'$1 > 1.0' >/dev/null")
assert not os.path.exists("./1.0")

def test_github_issue_492_3(self, bash):
"""Test code execution through unintended pathname expansions

When there is a file named "quote=$(COMMAND)" (for _filedir) or
"ret=$(COMMAND)" (for quote_readline), the completion of the word '$*
results in the execution of COMMAND.

$ echo '$*[TAB]

"""
os.mkdir("./ret=$(echo injected >&2)")
assert_bash_exec(bash, "quote_readline $'\\'$*' >/dev/null")


@pytest.mark.bashcomp(cmd=None, temp_cwd=True, pre_cmds=("shopt -s failglob",))
class TestUnitQuoteReadlineWithFailglob:
def test_github_issue_492_4(self, bash):
"""Test error messages through unintended pathname expansions

When "shopt -s failglob" is set by the user, the completion of the word
containing glob character and special characters (e.g. TAB) results in
the failure of pathname expansions.

$ shopt -s failglob
$ echo a\\ b*[TAB]

"""
assert_bash_exec(bash, "quote_readline $'a\\\\\\tb*' >/dev/null")