From 404b2864efa89f97f3bb14bf1448461d0c9de52f Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Fri, 27 Jan 2017 01:14:34 -0600 Subject: [PATCH] Additional lock down of conditionals --- lib/ansible/playbook/conditional.py | 70 ++++++++++++++++++----------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index 79369cef120..aa0926eb532 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -29,6 +29,7 @@ from ansible.compat.six import text_type from ansible.errors import AnsibleError, AnsibleUndefinedVariable from ansible.playbook.attribute import FieldAttribute from ansible.template import Templar +from ansible.template.safe_eval import safe_eval from ansible.module_utils._text import to_native DEFINED_REGEX = re.compile(r'(hostvars\[.+\]|[\w_]+)\s+(not\s+is|is|is\s+not)\s+(defined|undefined)') @@ -104,7 +105,9 @@ class Conditional: if not self._check_conditional(conditional, templar, all_vars): return False except Exception as e: - raise AnsibleError("The conditional check '%s' failed. The error was: %s" % (to_native(conditional), to_native(e)), obj=ds) + raise AnsibleError( + "The conditional check '%s' failed. The error was: %s" % (to_native(conditional), to_native(e)), obj=ds + ) return True @@ -143,35 +146,50 @@ class Conditional: # and we don't want future templating calls to do unsafe things disable_lookups |= hasattr(conditional, '__UNSAFE__') - # now we generated the "presented" string, which is a jinja2 if/else block - # used to evaluate the conditional. First, we do some low-level jinja2 parsing - # involving the AST format of the statement to ensure we don't do anything - # unsafe (using the disable_lookup flag above) - e = templar.environment.overlay() - e.filters.update(templar._get_filters()) - e.tests.update(templar._get_tests()) - - presented = "{%% if %s %%} True {%% else %%} False {%% endif %%}" % conditional - res = e._parse(presented, None, None) - res = generate(res, e, None, None) - parsed = ast.parse(res, mode='exec') - + # First, we do some low-level jinja2 parsing involving the AST format of the + # statement to ensure we don't do anything unsafe (using the disable_lookup flag above) class CleansingNodeVisitor(ast.NodeVisitor): - def generic_visit(self, node, inside_call=False): + def generic_visit(self, node, inside_call=False, inside_yield=False): if isinstance(node, ast.Call): inside_call = True + elif isinstance(node, ast.Yield): + inside_yield = True elif isinstance(node, ast.Str): - # calling things with a dunder is generally bad at this point... - if inside_call and disable_lookups and node.s.startswith("__"): - raise AnsibleError("Invalid access found in the presented conditional: '%s'" % conditional) + if disable_lookups: + if inside_call and node.s.startswith("__"): + # calling things with a dunder is generally bad at this point... + raise AnsibleError( + "Invalid access found in the conditional: '%s'" % conditional + ) + elif inside_yield: + # we're inside a yield, so recursively parse and traverse the AST + # of the result to catch forbidden syntax from executing + parsed = ast.parse(node.s, mode='exec') + cnv = CleansingNodeVisitor() + cnv.visit(parsed) # iterate over all child nodes for child_node in ast.iter_child_nodes(node): - self.generic_visit(child_node, inside_call=inside_call) + self.generic_visit( + child_node, + inside_call=inside_call, + inside_yield=inside_yield + ) + try: + e = templar.environment.overlay() + e.filters.update(templar._get_filters()) + e.tests.update(templar._get_tests()) + + res = e._parse(conditional, None, None) + res = generate(res, e, None, None) + parsed = ast.parse(res, mode='exec') - cnv = CleansingNodeVisitor() - cnv.visit(parsed) + cnv = CleansingNodeVisitor() + cnv.visit(parsed) + except Exception as e: + raise AnsibleError("Invalid conditional detected: %s" % to_native(e)) - # and finally we templated the presented string and look at the resulting string + # and finally we generate and template the presented string and look at the resulting string + presented = "{%% if %s %%} True {%% else %%} False {%% endif %%}" % conditional val = templar.template(presented, disable_lookups=disable_lookups).strip() if val == "True": return True @@ -180,8 +198,8 @@ class Conditional: else: raise AnsibleError("unable to evaluate conditional: %s" % original) except (AnsibleUndefinedVariable, UndefinedError) as e: - # the templating failed, meaning most likely a variable was undefined. If we happened to be - # looking for an undefined variable, return True, otherwise fail + # the templating failed, meaning most likely a variable was undefined. If we happened + # to be looking for an undefined variable, return True, otherwise fail try: # first we extract the variable name from the error message var_name = re.compile(r"'(hostvars\[.+\]|[\w_]+)' is undefined").search(str(e)).groups()[0] @@ -206,5 +224,7 @@ class Conditional: # trigger the AnsibleUndefinedVariable exception again below raise except Exception as new_e: - raise AnsibleUndefinedVariable("error while evaluating conditional (%s): %s" % (original, e)) + raise AnsibleUndefinedVariable( + "error while evaluating conditional (%s): %s" % (original, e) + )