From ed56f51f185a1ffd7ea57130d260098686fcc7c2 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Mon, 8 May 2017 10:37:10 -0500 Subject: [PATCH] Fixing security issue with lookup returns not tainting the jinja2 environment CVE-2017-7481 Lookup returns wrap the result in unsafe, however when used through the standard templar engine, this does not result in the jinja2 environment being marked as unsafe as a whole. This means the lookup result looses the unsafe protection and may become simple unicode strings, which can result in bad things being re-templated. This also adds a global lookup param and cfg options for lookups to allow unsafe returns, so users can force the previous (insecure) behavior. --- docs/docsite/rst/intro_configuration.rst | 14 ++++++++++++++ examples/ansible.cfg | 8 +++++++- lib/ansible/constants.py | 1 + lib/ansible/template/__init__.py | 11 +++++++++-- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/docs/docsite/rst/intro_configuration.rst b/docs/docsite/rst/intro_configuration.rst index 3647e22..259e107 100644 --- a/docs/docsite/rst/intro_configuration.rst +++ b/docs/docsite/rst/intro_configuration.rst @@ -86,6 +86,20 @@ different locations:: Most users will not need to use this feature. See :doc:`dev_guide/developing_plugins` for more details. +.. _allow_unsafe_lookups: + +allow_unsafe_lookups +==================== + +.. versionadded:: 2.2.3, 2.3.1 + +When enabled, this option allows lookup plugins (whether used in variables as `{{lookup('foo')}}` or as a loop as `with_foo`) to return data that is **not** marked "unsafe". By default, such data is marked as unsafe to prevent the templating engine from evaluating any jinja2 templating language, as this could represent a security risk. + +This option is provided to allow for backwards-compatibility, however users should first consider adding `allow_unsafe=True` to any lookups which may be expected to contain data which may be run through the templating engine later. For example:: + + {{lookup('pipe', '/path/to/some/command', allow_unsafe=True)}} + + .. _allow_world_readable_tmpfiles: allow_world_readable_tmpfiles diff --git a/examples/ansible.cfg b/examples/ansible.cfg index e283064..77ba5d2 100644 --- a/examples/ansible.cfg +++ b/examples/ansible.cfg @@ -282,7 +282,7 @@ # Controls showing custom stats at the end, off by default #show_custom_stats = True -# Controlls which files to ignore when using a directory as inventory with +# Controls which files to ignore when using a directory as inventory with # possibly multiple sources (both static and dynamic) #inventory_ignore_extensions = ~, .orig, .bak, .ini, .cfg, .retry, .pyc, .pyo @@ -294,6 +294,12 @@ # Setting to True keeps them under the ansible_facts namespace, the default is False #restrict_facts_namespace: True +# When enabled, this option allows lookups (via variables like {{lookup('foo')}} or when used as +# a loop with `with_foo`) to return data that is not marked "unsafe". This means the data may contain +# jinja2 templating language which will be run through the templating engine. +# ENABLING THIS COULD BE A SECURITY RISK +#allow_unsafe_lookups = False + [privilege_escalation] #become=True #become_method=sudo diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index da45037..40d1038 100644 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@ -236,6 +236,7 @@ def load_config_file(): ["~", ".orig", ".bak", ".ini", ".cfg", ".retry", ".pyc", ".pyo"], value_type='list') DEFAULT_VAR_COMPRESSION_LEVEL = get_config(p, DEFAULTS, 'var_compression_level', 'ANSIBLE_VAR_COMPRESSION_LEVEL', 0, value_type='integer') DEFAULT_INTERNAL_POLL_INTERVAL = get_config(p, DEFAULTS, 'internal_poll_interval', None, 0.001, value_type='float') +DEFAULT_ALLOW_UNSAFE_LOOKUPS = get_config(p, DEFAULTS, 'allow_unsafe_lookups', None, False, value_type='boolean') ERROR_ON_MISSING_HANDLER = get_config(p, DEFAULTS, 'error_on_missing_handler', 'ANSIBLE_ERROR_ON_MISSING_HANDLER', True, value_type='boolean') SHOW_CUSTOM_STATS = get_config(p, DEFAULTS, 'show_custom_stats', 'ANSIBLE_SHOW_CUSTOM_STATS', False, value_type='boolean') NAMESPACE_FACTS = get_config(p, DEFAULTS, 'restrict_facts_namespace', 'ANSIBLE_RESTRICT_FACTS', False, value_type='boolean') diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 5d551d7..49de8aa 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -252,6 +252,9 @@ def __init__(self, loader, shared_loader_obj=None, variables=dict()): loader=FileSystemLoader(self._basedir), ) + # the current rendering context under which the templar class is working + self.cur_context = None + self.SINGLE_VAR = re.compile(r"^%s\s*(\w*)\s*%s$" % (self.environment.variable_start_string, self.environment.variable_end_string)) self._clean_regex = re.compile(r'(?:%s|%s|%s|%s)' % ( @@ -574,6 +577,7 @@ def _lookup(self, name, *args, **kwargs): if instance is not None: wantlist = kwargs.pop('wantlist', False) + allow_unsafe = kwargs.pop('allow_unsafe', C.DEFAULT_ALLOW_UNSAFE_LOOKUPS) from ansible.utils.listify import listify_lookup_plugin_terms loop_terms = listify_lookup_plugin_terms(terms=args, templar=self, loader=self._loader, fail_on_undefined=True, convert_bare=False) @@ -510,7 +510,7 @@ raise AnsibleError("An unhandled exception occurred while running the lookup plugin '%s'. Error was a %s, original message: %s" % (name, type(e), e)) ran = None - if ran: + if ran and not allow_unsafe: from ansible.vars.unsafe_proxy import UnsafeProxy, wrap_var if wantlist: ran = wrap_var(ran) @@ -600,6 +605,8 @@ def _lookup(self, name, *args, **kwargs): else: ran = wrap_var(ran) + if self.cur_context: + self.cur_context.unsafe = True return ran else: raise AnsibleError("lookup plugin (%s) not found" % name) @@ -656,7 +663,7 @@ def do_template(self, data, preserve_trailing_newlines=True, escape_backslashes= jvars = AnsibleJ2Vars(self, t.globals) - new_context = t.new_context(jvars, shared=True) + self.cur_context = new_context = t.new_context(jvars, shared=True) rf = t.root_render_func(new_context) try: