Skip to content

Commit ed56f51

Browse files
committed
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.
1 parent 6f4f701 commit ed56f51

File tree

4 files changed

+31
-3
lines changed

4 files changed

+31
-3
lines changed

Diff for: docs/docsite/rst/intro_configuration.rst

+14
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ different locations::
8686
Most users will not need to use this feature. See :doc:`dev_guide/developing_plugins` for more details.
8787

8888

89+
.. _allow_unsafe_lookups:
90+
91+
allow_unsafe_lookups
92+
====================
93+
94+
.. versionadded:: 2.2.3, 2.3.1
95+
96+
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.
97+
98+
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::
99+
100+
{{lookup('pipe', '/path/to/some/command', allow_unsafe=True)}}
101+
102+
89103
.. _allow_world_readable_tmpfiles:
90104

91105
allow_world_readable_tmpfiles

Diff for: examples/ansible.cfg

+7-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@
282282
# Controls showing custom stats at the end, off by default
283283
#show_custom_stats = True
284284

285-
# Controlls which files to ignore when using a directory as inventory with
285+
# Controls which files to ignore when using a directory as inventory with
286286
# possibly multiple sources (both static and dynamic)
287287
#inventory_ignore_extensions = ~, .orig, .bak, .ini, .cfg, .retry, .pyc, .pyo
288288

@@ -294,6 +294,12 @@
294294
# Setting to True keeps them under the ansible_facts namespace, the default is False
295295
#restrict_facts_namespace: True
296296

297+
# When enabled, this option allows lookups (via variables like {{lookup('foo')}} or when used as
298+
# a loop with `with_foo`) to return data that is not marked "unsafe". This means the data may contain
299+
# jinja2 templating language which will be run through the templating engine.
300+
# ENABLING THIS COULD BE A SECURITY RISK
301+
#allow_unsafe_lookups = False
302+
297303
[privilege_escalation]
298304
#become=True
299305
#become_method=sudo

Diff for: lib/ansible/constants.py

+1
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ def load_config_file():
236236
["~", ".orig", ".bak", ".ini", ".cfg", ".retry", ".pyc", ".pyo"], value_type='list')
237237
DEFAULT_VAR_COMPRESSION_LEVEL = get_config(p, DEFAULTS, 'var_compression_level', 'ANSIBLE_VAR_COMPRESSION_LEVEL', 0, value_type='integer')
238238
DEFAULT_INTERNAL_POLL_INTERVAL = get_config(p, DEFAULTS, 'internal_poll_interval', None, 0.001, value_type='float')
239+
DEFAULT_ALLOW_UNSAFE_LOOKUPS = get_config(p, DEFAULTS, 'allow_unsafe_lookups', None, False, value_type='boolean')
239240
ERROR_ON_MISSING_HANDLER = get_config(p, DEFAULTS, 'error_on_missing_handler', 'ANSIBLE_ERROR_ON_MISSING_HANDLER', True, value_type='boolean')
240241
SHOW_CUSTOM_STATS = get_config(p, DEFAULTS, 'show_custom_stats', 'ANSIBLE_SHOW_CUSTOM_STATS', False, value_type='boolean')
241242
NAMESPACE_FACTS = get_config(p, DEFAULTS, 'restrict_facts_namespace', 'ANSIBLE_RESTRICT_FACTS', False, value_type='boolean')

Diff for: lib/ansible/template/__init__.py

+9-2
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,9 @@ def __init__(self, loader, shared_loader_obj=None, variables=dict()):
252252
loader=FileSystemLoader(self._basedir),
253253
)
254254

255+
# the current rendering context under which the templar class is working
256+
self.cur_context = None
257+
255258
self.SINGLE_VAR = re.compile(r"^%s\s*(\w*)\s*%s$" % (self.environment.variable_start_string, self.environment.variable_end_string))
256259

257260
self._clean_regex = re.compile(r'(?:%s|%s|%s|%s)' % (
@@ -574,6 +577,7 @@ def _lookup(self, name, *args, **kwargs):
574577

575578
if instance is not None:
576579
wantlist = kwargs.pop('wantlist', False)
580+
allow_unsafe = kwargs.pop('allow_unsafe', C.DEFAULT_ALLOW_UNSAFE_LOOKUPS)
577581

578582
from ansible.utils.listify import listify_lookup_plugin_terms
579583
loop_terms = listify_lookup_plugin_terms(terms=args, templar=self, loader=self._loader, fail_on_undefined=True, convert_bare=False)
@@ -588,7 +592,8 @@ def _lookup(self, name, *args, **kwargs):
588592
"original message: %s" % (name, type(e), e))
589593
ran = None
590594

591-
if ran:
595+
if ran and not allow_unsafe:
596+
from ansible.vars.unsafe_proxy import UnsafeProxy, wrap_var
592597
if wantlist:
593598
ran = wrap_var(ran)
594599
else:
@@ -600,6 +605,8 @@ def _lookup(self, name, *args, **kwargs):
600605
else:
601606
ran = wrap_var(ran)
602607

608+
if self.cur_context:
609+
self.cur_context.unsafe = True
603610
return ran
604611
else:
605612
raise AnsibleError("lookup plugin (%s) not found" % name)
@@ -656,7 +663,7 @@ def do_template(self, data, preserve_trailing_newlines=True, escape_backslashes=
656663

657664
jvars = AnsibleJ2Vars(self, t.globals)
658665

659-
new_context = t.new_context(jvars, shared=True)
666+
self.cur_context = new_context = t.new_context(jvars, shared=True)
660667
rf = t.root_render_func(new_context)
661668

662669
try:

0 commit comments

Comments
 (0)