Fix CVE-2024-11079 hostvars unsafe context (#84339) (#84353)

Fix to preserve an unsafe variable when accessing through an
intermediary variable from hostvars.

(cherry picked from commit 2936b80)

Author: jborean93

changelogs/fragments/unsafe_hostvars_fix.yml

@@ -0,0 +1,2 @@
+security_fixes:
+  - Templating will not prefer AnsibleUnsafe when a variable is referenced via hostvars - CVE-2024-11079

lib/ansible/template/__init__.py

@@ -49,7 +49,7 @@
 from ansible.module_utils.common.text.converters import to_native, to_text, to_bytes
 from ansible.module_utils.common.collections import is_sequence
 from ansible.plugins.loader import filter_loader, lookup_loader, test_loader
-from ansible.template.native_helpers import ansible_native_concat, ansible_eval_concat, ansible_concat
+from ansible.template.native_helpers import AnsibleUndefined, ansible_native_concat, ansible_eval_concat, ansible_concat
 from ansible.template.template import AnsibleJ2Template
 from ansible.template.vars import AnsibleJ2Vars
 from ansible.utils.display import Display
@@ -329,35 +329,6 @@ def wrapper(*args, **kwargs):
     return _update_wrapper(wrapper, func)
 
 
-class AnsibleUndefined(StrictUndefined):
-    '''
-    A custom Undefined class, which returns further Undefined objects on access,
-    rather than throwing an exception.
-    '''
-    def __getattr__(self, name):
-        if name == '__UNSAFE__':
-            # AnsibleUndefined should never be assumed to be unsafe
-            # This prevents ``hasattr(val, '__UNSAFE__')`` from evaluating to ``True``
-            raise AttributeError(name)
-        # Return original Undefined object to preserve the first failure context
-        return self
-
-    def __getitem__(self, key):
-        # Return original Undefined object to preserve the first failure context
-        return self
-
-    def __repr__(self):
-        return 'AnsibleUndefined(hint={0!r}, obj={1!r}, name={2!r})'.format(
-            self._undefined_hint,
-            self._undefined_obj,
-            self._undefined_name
-        )
-
-    def __contains__(self, item):
-        # Return original Undefined object to preserve the first failure context
-        return self
-
-
 class AnsibleContext(Context):
     '''
     A custom context, which intercepts resolve_or_missing() calls and sets a flag

lib/ansible/template/native_helpers.py

@@ -7,13 +7,19 @@
 
 
 import ast
+from collections.abc import Mapping
 from itertools import islice, chain
 from types import GeneratorType
 
+from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.common.text.converters import to_text
 from ansible.module_utils.six import string_types
 from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode
 from ansible.utils.native_jinja import NativeJinjaText
+from ansible.utils.unsafe_proxy import wrap_var
+import ansible.module_utils.compat.typing as t
+
+from jinja2.runtime import StrictUndefined
 
 
 _JSON_MAP = {
@@ -30,6 +36,40 @@ def visit_Name(self, node):
         return ast.Constant(value=_JSON_MAP[node.id])
 
 
+def _is_unsafe(value: t.Any) -> bool:
+    """
+    Our helper function, which will also recursively check dict and
+    list entries due to the fact that they may be repr'd and contain
+    a key or value which contains jinja2 syntax and would otherwise
+    lose the AnsibleUnsafe value.
+    """
+    to_check = [value]
+    seen = set()
+
+    while True:
+        if not to_check:
+            break
+
+        val = to_check.pop(0)
+        val_id = id(val)
+
+        if val_id in seen:
+            continue
+        seen.add(val_id)
+
+        if isinstance(val, AnsibleUndefined):
+            continue
+        if isinstance(val, Mapping):
+            to_check.extend(val.keys())
+            to_check.extend(val.values())
+        elif is_sequence(val):
+            to_check.extend(val)
+        elif getattr(val, '__UNSAFE__', False):
+            return True
+
+    return False
+
+
 def ansible_eval_concat(nodes):
     """Return a string of concatenated compiled nodes. Throw an undefined error
     if any of the nodes is undefined.
@@ -45,17 +85,28 @@ def ansible_eval_concat(nodes):
     if not head:
         return ''
 
+    unsafe = False
+
     if len(head) == 1:
         out = head[0]
 
         if isinstance(out, NativeJinjaText):
             return out
 
+        unsafe = _is_unsafe(out)
         out = to_text(out)
     else:
         if isinstance(nodes, GeneratorType):
             nodes = chain(head, nodes)
-        out = ''.join([to_text(v) for v in nodes])
+
+        out_values = []
+        for v in nodes:
+            if not unsafe and _is_unsafe(v):
+                unsafe = True
+
+            out_values.append(to_text(v))
+
+        out = ''.join(out_values)
 
     # if this looks like a dictionary, list or bool, convert it to such
     if out.startswith(('{', '[')) or out in ('True', 'False'):
@@ -70,6 +121,9 @@ def ansible_eval_concat(nodes):
         except (TypeError, ValueError, SyntaxError, MemoryError):
             pass
 
+    if unsafe:
+        out = wrap_var(out)
+
     return out
 
 
@@ -80,7 +134,19 @@ def ansible_concat(nodes):
 
     Used in Templar.template() when jinja2_native=False and convert_data=False.
     """
-    return ''.join([to_text(v) for v in nodes])
+    unsafe = False
+    values = []
+    for v in nodes:
+        if not unsafe and _is_unsafe(v):
+            unsafe = True
+
+        values.append(to_text(v))
+
+    out = ''.join(values)
+    if unsafe:
+        out = wrap_var(out)
+
+    return out
 
 
 def ansible_native_concat(nodes):
@@ -97,6 +163,8 @@ def ansible_native_concat(nodes):
     if not head:
         return None
 
+    unsafe = False
+
     if len(head) == 1:
         out = head[0]
 
@@ -117,10 +185,21 @@ def ansible_native_concat(nodes):
         # short-circuit literal_eval for anything other than strings
         if not isinstance(out, string_types):
             return out
+
+        unsafe = _is_unsafe(out)
+
     else:
         if isinstance(nodes, GeneratorType):
             nodes = chain(head, nodes)
-        out = ''.join([to_text(v) for v in nodes])
+
+        out_values = []
+        for v in nodes:
+            if not unsafe and _is_unsafe(v):
+                unsafe = True
+
+            out_values.append(to_text(v))
+
+        out = ''.join(out_values)
 
     try:
         evaled = ast.literal_eval(
@@ -130,10 +209,45 @@ def ansible_native_concat(nodes):
             ast.parse(out, mode='eval')
         )
     except (TypeError, ValueError, SyntaxError, MemoryError):
+        if unsafe:
+            out = wrap_var(out)
+
         return out
 
     if isinstance(evaled, string_types):
         quote = out[0]
-        return f'{quote}{evaled}{quote}'
+        evaled = f'{quote}{evaled}{quote}'
+
+    if unsafe:
+        evaled = wrap_var(evaled)
 
     return evaled
+
+
+class AnsibleUndefined(StrictUndefined):
+    """
+    A custom Undefined class, which returns further Undefined objects on access,
+    rather than throwing an exception.
+    """
+    def __getattr__(self, name):
+        if name == '__UNSAFE__':
+            # AnsibleUndefined should never be assumed to be unsafe
+            # This prevents ``hasattr(val, '__UNSAFE__')`` from evaluating to ``True``
+            raise AttributeError(name)
+        # Return original Undefined object to preserve the first failure context
+        return self
+
+    def __getitem__(self, key):
+        # Return original Undefined object to preserve the first failure context
+        return self
+
+    def __repr__(self):
+        return 'AnsibleUndefined(hint={0!r}, obj={1!r}, name={2!r})'.format(
+            self._undefined_hint,
+            self._undefined_obj,
+            self._undefined_name
+        )
+
+    def __contains__(self, item):
+        # Return original Undefined object to preserve the first failure context
+        return self

lib/ansible/vars/hostvars.py

@@ -94,11 +94,12 @@ def __contains__(self, host_name):
         return self._find_host(host_name) is not None
 
     def __iter__(self):
-        for host in self._inventory.hosts:
-            yield host
+        # include implicit localhost only if it has variables set
+        yield from self._inventory.hosts | {'localhost': self._inventory.localhost} if self._inventory.localhost else {}
 
     def __len__(self):
-        return len(self._inventory.hosts)
+        # include implicit localhost only if it has variables set
+        return len(self._inventory.hosts) + (1 if self._inventory.localhost else 0)
 
     def __repr__(self):
         out = {}

test/integration/targets/template/cve-2024-11079.yml

@@ -0,0 +1,30 @@
+- name: test CVE-2024-11079 loop variables preserve unsafe hostvars
+  hosts: localhost
+  gather_facts: false
+  tasks:
+  - set_fact:
+      foo:
+        safe:
+          prop: '{{ "{{" }} unsafe_var {{ "}}" }}'
+        unsafe:
+          prop: !unsafe '{{ unsafe_var }}'
+
+  - name: safe var through hostvars loop is templated
+    assert:
+      that:
+      - item.prop == expected
+    loop:
+    - "{{ hostvars['localhost']['foo']['safe'] }}"
+    vars:
+      unsafe_var: bar
+      expected: bar
+
+  - name: unsafe var through hostvars loop is not templated
+    assert:
+      that:
+      - item.prop == expected
+    loop:
+    - "{{ hostvars['localhost']['foo']['unsafe'] }}"
+    vars:
+      unsafe_var: bar
+      expected: !unsafe '{{ unsafe_var }}'

test/integration/targets/template/runme.sh

@@ -41,6 +41,10 @@ ansible-playbook 72262.yml -v "$@"
 # ensure unsafe is preserved, even with extra newlines
 ansible-playbook unsafe.yml -v "$@"
 
+# CVE 2024-11079
+ANSIBLE_JINJA2_NATIVE=true ansible-playbook cve-2024-11079.yml -v "$@"
+ANSIBLE_JINJA2_NATIVE=false ansible-playbook cve-2024-11079.yml -v "$@"
+
 # ensure Jinja2 overrides from a template are used
 ansible-playbook template_overrides.yml -v "$@"