From 5a5b9f211b611f74958bc3f72efaf7e7da6ae745 Mon Sep 17 00:00:00 2001
From: James Cammarata <jimi@sngx.net>
Date: Fri, 28 Aug 2015 11:35:43 -0400
Subject: [PATCH] Validate variable names when loading 'vars:' blocks

TODO: add this to VariableManager to validate vars loaded from files too

Fixes #12022
---
 lib/ansible/playbook/base.py           | 11 +++++++-
 lib/ansible/plugins/action/set_fact.py | 35 ++------------------------
 lib/ansible/utils/vars.py              | 35 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py
index 3dca143a54..d1cb5f07a1 100644
--- a/lib/ansible/playbook/base.py
+++ b/lib/ansible/playbook/base.py
@@ -37,7 +37,7 @@ from ansible.playbook.attribute import Attribute, FieldAttribute
 from ansible.template import Templar
 from ansible.utils.boolean import boolean
 from ansible.utils.debug import debug
-from ansible.utils.vars import combine_vars
+from ansible.utils.vars import combine_vars, isidentifier
 from ansible.template import template
 
 class Base:
@@ -379,14 +379,21 @@ class Base:
         list into a single dictionary.
         '''
 
+        def _validate_variable_keys(ds):
+            for key in ds:
+                if not isidentifier(key):
+                    raise TypeError("%s is not a valid variable name" % key)
+
         try:
             if isinstance(ds, dict):
+                _validate_variable_keys(ds)
                 return ds
             elif isinstance(ds, list):
                 all_vars = dict()
                 for item in ds:
                     if not isinstance(item, dict):
                         raise ValueError
+                    _validate_variable_keys(item)
                     all_vars = combine_vars(all_vars, item)
                 return all_vars
             elif ds is None:
@@ -395,6 +402,8 @@ class Base:
                 raise ValueError
         except ValueError:
             raise AnsibleParserError("Vars in a %s must be specified as a dictionary, or a list of dictionaries" % self.__class__.__name__, obj=ds)
+        except TypeError, e:
+            raise AnsibleParserError("Invalid variable name in vars specified for %s: %s" % (self.__class__.__name__, e), obj=ds)
 
     def _extend_value(self, value, new_value):
         '''
diff --git a/lib/ansible/plugins/action/set_fact.py b/lib/ansible/plugins/action/set_fact.py
index 5822fb3f08..34f4078ac1 100644
--- a/lib/ansible/plugins/action/set_fact.py
+++ b/lib/ansible/plugins/action/set_fact.py
@@ -14,47 +14,16 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import ast
-
-from six import string_types
 
 from ansible.errors import AnsibleError
 from ansible.plugins.action import ActionBase
 from ansible.utils.boolean import boolean
+from ansible.utils.vars import isidentifier
 
-def isidentifier(ident):
-    """
-    Determines, if string is valid Python identifier using the ast module.
-    Orignally posted at: http://stackoverflow.com/a/29586366
-    """
-
-    if not isinstance(ident, string_types):
-        return False
-
-    try:
-        root = ast.parse(ident)
-    except SyntaxError:
-        return False
-
-    if not isinstance(root, ast.Module):
-        return False
-
-    if len(root.body) != 1:
-        return False
-
-    if not isinstance(root.body[0], ast.Expr):
-        return False
-
-    if not isinstance(root.body[0].value, ast.Name):
-        return False
-
-    if root.body[0].value.id != ident:
-        return False
-
-    return True
 
 class ActionModule(ActionBase):
 
diff --git a/lib/ansible/utils/vars.py b/lib/ansible/utils/vars.py
index bfbc9d1a82..d53473412f 100644
--- a/lib/ansible/utils/vars.py
+++ b/lib/ansible/utils/vars.py
@@ -19,6 +19,9 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import ast
+
+from six import string_types
 
 from ansible import constants as C
 from ansible.parsing.splitter import parse_kv
@@ -66,3 +69,35 @@ def load_extra_vars(loader, options):
             data = parse_kv(extra_vars_opt)
         extra_vars = combine_vars(extra_vars, data)
     return extra_vars
+
+def isidentifier(ident):
+    """
+    Determines, if string is valid Python identifier using the ast module.
+    Orignally posted at: http://stackoverflow.com/a/29586366
+    """
+
+    if not isinstance(ident, string_types):
+        return False
+
+    try:
+        root = ast.parse(ident)
+    except SyntaxError:
+        return False
+
+    if not isinstance(root, ast.Module):
+        return False
+
+    if len(root.body) != 1:
+        return False
+
+    if not isinstance(root.body[0], ast.Expr):
+        return False
+
+    if not isinstance(root.body[0].value, ast.Name):
+        return False
+
+    if root.body[0].value.id != ident:
+        return False
+
+    return True
+