[reprotest] 01/01: Properly sanitize artifact_pattern to avoid arbitrary shell execution
Ximin Luo
infinity0 at debian.org
Fri Sep 15 15:15:59 UTC 2017
This is an automated email from the git hooks/post-receive script.
infinity0 pushed a commit to branch master
in repository reprotest.
commit f8e260bf94fa7fea21ce45ae24c852c60c71b2d1
Author: Ximin Luo <infinity0 at debian.org>
Date: Fri Sep 15 17:13:57 2017 +0200
Properly sanitize artifact_pattern to avoid arbitrary shell execution
---
debian/changelog | 1 +
reprotest/__init__.py | 7 ++--
reprotest/shell_syn.py | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/test_shell.py | 37 ++++++++++++++++++++
4 files changed, 135 insertions(+), 3 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 82d4217..75ad952 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -12,6 +12,7 @@ reprotest (0.7) UNRELEASED; urgency=medium
different variations, and to make it easier to configure further builds.
* Deprecate the --dont-vary flag, add a --vary flag for better composability.
* Support >2 builds using the new --extra-build flag.
+ * Properly sanitize artifact_pattern to avoid arbitrary shell execution.
[ Mattia Rizzolo ]
* Bump Standards-Version to 4.0.0.
diff --git a/reprotest/__init__.py b/reprotest/__init__.py
index 16b4fc9..9ef5f18 100644
--- a/reprotest/__init__.py
+++ b/reprotest/__init__.py
@@ -21,7 +21,7 @@ import pkg_resources
from reprotest.lib import adtlog
from reprotest.lib import adt_testbed
from reprotest.build import Build, VariationSpec, Variations
-from reprotest import presets
+from reprotest import presets, shell_syn
VIRT_PREFIX = "autopkgtest-virt-"
@@ -127,8 +127,6 @@ class BuildContext(collections.namedtuple('_BuildContext', 'testbed_root local_d
self.testbed_src, artifact_pattern)
# remove any existing artifact, in case the build script doesn't overwrite
# it e.g. like how make(1) sometimes works.
- if re.search(r"""(^| )['"]*/""", artifact_pattern):
- raise ValueError("artifact_pattern is possibly dangerous; maybe use a relative path instead?")
testbed.check_exec(
['sh', '-ec', 'cd "%s" && rm -rf %s' %
(self.testbed_src, artifact_pattern)])
@@ -184,6 +182,9 @@ def check(build_command, artifact_pattern, virtual_server_args, source_root,
if os.path.isfile(source_root):
source_root = os.path.normpath(os.path.dirname(source_root))
+ artifact_pattern = shell_syn.sanitize_globs(artifact_pattern)
+ logging.debug("artifact_pattern sanitized to: %s", artifact_pattern)
+
if store_dir:
store_dir = str(store_dir)
if not os.path.exists(store_dir):
diff --git a/reprotest/shell_syn.py b/reprotest/shell_syn.py
new file mode 100644
index 0000000..98a6ab5
--- /dev/null
+++ b/reprotest/shell_syn.py
@@ -0,0 +1,93 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/gpl-3.0.en.html
+# For details: reprotest/debian/copyright
+
+# http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
+# Special chars except allowed ones <space> " ' \
+special_except_quotes=r"""
+ |&;<>()$`"""
+special_in_double_quotes=r"$`"
+escaped_in_double_quotes=r"""$`"\
+"""
+
+def sanitize_globs(s, join=True, forcerel=True):
+ """Ensure that a shell snippet only contains glob expressions, and nothing
+ that can be executed.
+
+ Args:
+
+ s: The snippet to sanitize
+ join: If true, returns a single shell snippet string, else a list of shell
+ snippet words (that include quotes and other limit) that may be joined
+ by a single space into a single shell snippet. Default: true.
+ forcerel: If true, prepends ./ to all snippet words. This ensures that they
+ can't be accidentally interpreted as options to most commands.
+ """
+ # output state
+ words = []
+ cw = None
+ def next_word():
+ nonlocal cw, words
+ if cw is not None:
+ words.append(cw)
+ cw = None
+
+ # parse state
+ in_quote = False
+ escaped = False
+
+ for c in s:
+ #print(c, in_quote, escaped)
+ if not in_quote:
+ if escaped:
+ escaped = False
+ else:
+ if c in special_except_quotes:
+ raise ValueError("not a shell-glob pattern: %s" % s)
+ elif c == "\\":
+ escaped = True
+ elif c in "'\"":
+ in_quote = c
+ elif c == " ":
+ next_word()
+ continue
+
+ elif in_quote == "'":
+ if c == "'":
+ in_quote = False
+
+ elif in_quote == "\"":
+ if escaped:
+ if c not in escaped_in_double_quotes:
+ # as per the spec, these chars retain the backslash, so
+ # append that to cw before we append c as described below
+ cw = "\\" if cw is None else cw + "\\"
+ escaped = False
+ else:
+ if c in special_in_double_quotes:
+ raise ValueError("not a shell-glob pattern: %s" % s)
+ elif c == "\\":
+ escaped = True
+ elif c == "\"":
+ in_quote = False
+
+ else:
+ assert False
+
+ # append c onto cw. we do this uncondtionally because we're only
+ # sanitising the string and not parsing it; we want to keep it in a
+ # form that the shell can parse
+ cw = c if cw is None else cw + c
+
+ if in_quote or escaped:
+ raise ValueError("unclosed escape or quote: %s" % s)
+
+ next_word()
+
+ words = ["./" + w for w in words] if forcerel else words
+ return " ".join(words) if join else words
+
+
+if __name__ == "__main__":
+ import sys
+ for a in sys.argv[1:]:
+ print(sanitize_globs(a))
diff --git a/tests/test_shell.py b/tests/test_shell.py
new file mode 100644
index 0000000..1788412
--- /dev/null
+++ b/tests/test_shell.py
@@ -0,0 +1,37 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/gpl-3.0.en.html
+# For details: reprotest/debian/copyright
+
+import pytest
+
+import random
+import shlex
+import string
+
+from reprotest.shell_syn import *
+
+
+def test_basic():
+ assert (sanitize_globs("""lol \$ *"\$ s" "" '' ' ' " " ' ' "" """)
+ == '''./lol ./\$ ./*"\$ s" ./"" ./'' ./' ' ./" " ./' ' ./""''')
+ assert (sanitize_globs("""*"*"'*' ../hm wut???""")
+ == '''./*"*"'*' ./../hm ./wut???''')
+
+ with pytest.raises(ValueError):
+ sanitize_globs('; sudo rm -rf /')
+ with pytest.raises(ValueError):
+ sanitize_globs('`sudo rm -rf /`')
+ with pytest.raises(ValueError):
+ sanitize_globs('<(sudo rm -rf /)')
+ with pytest.raises(ValueError):
+ sanitize_globs('$(sudo rm -rf /)')
+ with pytest.raises(ValueError):
+ sanitize_globs('ok; sudo rm -rf /')
+
+ assert sanitize_globs('-rf') == './-rf'
+ assert sanitize_globs('/') == './/'
+
+def test_shlex_quote():
+ for _ in range(65536):
+ x = ''.join(random.choice(string.printable) for _ in range(random.randint(0, 32)))
+ assert len(sanitize_globs(shlex.quote(x), False)) == 1
+ assert len(shlex.split(sanitize_globs(shlex.quote(x)))) == 1
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/reprotest.git
More information about the Reproducible-commits
mailing list