Additional lock down of conditionals

(cherry picked from commit 9751bf440e2b026874e70f950920e6dbee2e9115)

lib/ansible/playbook/conditional.py

@@ -127,35 +127,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" % 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
@@ -164,8 +179,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]
@@ -190,5 +205,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)
+                )