Skip to content

Commit

Permalink
Fixing security issue with lookup returns not tainting the jinja2 env…
Browse files Browse the repository at this point in the history
…ironment

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.

(cherry picked from commit 72dfb1570d22ac519350a8c09e76c458789120ed)
  • Loading branch information
jimi-c committed May 8, 2017
1 parent ab39914 commit a188691
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 3 deletions.
14 changes: 14 additions & 0 deletions docs/docsite/rst/intro_configuration.rst
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion examples/ansible.cfg
Expand Up @@ -281,14 +281,20 @@
# 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

# This family of modules use an alternative execution path optimized for network appliances
# only update this setting if you know how this works, otherwise it can break module execution
#network_group_modules=['eos', 'nxos', 'ios', 'iosxr', 'junos', 'vyos']

# 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
Expand Down
1 change: 1 addition & 0 deletions lib/ansible/constants.py
Expand Up @@ -234,6 +234,7 @@ def load_config_file():
DEFAULT_INVENTORY_IGNORE = get_config(p, DEFAULTS, 'inventory_ignore_extensions', 'ANSIBLE_INVENTORY_IGNORE', ["~", ".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')

Expand Down
11 changes: 9 additions & 2 deletions lib/ansible/template/__init__.py
Expand Up @@ -251,6 +251,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.block_start = self.environment.block_start_string
Expand Down Expand Up @@ -564,6 +567,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)
Expand All @@ -577,7 +581,8 @@ def _lookup(self, name, *args, **kwargs):
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)
else:
Expand All @@ -589,6 +594,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)
Expand Down Expand Up @@ -645,7 +652,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:
Expand Down

0 comments on commit a188691

Please sign in to comment.