[stable-2.14] Prevent roles from using symlinks to overwrite files outside of the installation directory (#81780) (#81786)

* Sanitize linkname during role installs

* Add tests

* add clog frag.
(cherry picked from commit ddf0311c63)

changelogs/fragments/cve-2023-5115.yml

@@ -0,0 +1,3 @@
+security_fixes:
+- ansible-galaxy - Prevent roles from using symlinks to overwrite
+  files outside of the installation directory (CVE-2023-5115)

lib/ansible/galaxy/role.py

@@ -394,18 +394,36 @@ class GalaxyRole(object):
                             # bits that might be in the file for security purposes
                             # and drop any containing directory, as mentioned above
                             if member.isreg() or member.issym():
-                                n_member_name = to_native(member.name)
+                                for attr in ('name', 'linkname'):
-                                n_archive_parent_dir = to_native(archive_parent_dir)
+                                    attr_value = getattr(member, attr, None)
-                                n_parts = n_member_name.replace(n_archive_parent_dir, "", 1).split(os.sep)
+                                    if not attr_value:
-                                n_final_parts = []
+                                        continue
-                                for n_part in n_parts:
+                                    n_attr_value = to_native(attr_value)
-                                    # TODO if the condition triggers it produces a broken installation.
+                                    n_archive_parent_dir = to_native(archive_parent_dir)
-                                    # It will create the parent directory as an empty file and will
+                                    n_parts = n_attr_value.replace(n_archive_parent_dir, "", 1).split(os.sep)
-                                    # explode if the directory contains valid files.
+                                    n_final_parts = []
-                                    # Leaving this as is since the whole module needs a rewrite.
+                                    for n_part in n_parts:
-                                    if n_part != '..' and not n_part.startswith('~') and '$' not in n_part:
+                                        # TODO if the condition triggers it produces a broken installation.
+                                        # It will create the parent directory as an empty file and will
+                                        # explode if the directory contains valid files.
+                                        # Leaving this as is since the whole module needs a rewrite.
+                                        #
+                                        # Check if we have any files with illegal names,
+                                        # and display a warning if so. This could help users
+                                        # to debug a broken installation.
+                                        if not n_part:
+                                            continue
+                                        if n_part == '..':
+                                            display.warning(f"Illegal filename '{n_part}': '..' is not allowed")
+                                            continue
+                                        if n_part.startswith('~'):
+                                            display.warning(f"Illegal filename '{n_part}': names cannot start with '~'")
+                                            continue
+                                        if '$' in n_part:
+                                            display.warning(f"Illegal filename '{n_part}': names cannot contain '$'")
+                                            continue
                                         n_final_parts.append(n_part)
-                                member.name = os.path.join(*n_final_parts)
+                                    setattr(member, attr, os.path.join(*n_final_parts))
 
                                 if _check_working_data_filter():
                                     # deprecated: description='extract fallback without filter' python_version='3.11'

test/integration/targets/ansible-galaxy-role/files/create-role-archive.py

@@ -0,0 +1,45 @@
+#!/usr/bin/env python
+"""Create a role archive which overwrites an arbitrary file."""
+
+import argparse
+import pathlib
+import tarfile
+import tempfile
+
+
+def main() -> None:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('archive', type=pathlib.Path, help='archive to create')
+    parser.add_argument('content', type=pathlib.Path, help='content to write')
+    parser.add_argument('target', type=pathlib.Path, help='file to overwrite')
+
+    args = parser.parse_args()
+
+    create_archive(args.archive, args.content, args.target)
+
+
+def create_archive(archive_path: pathlib.Path, content_path: pathlib.Path, target_path: pathlib.Path) -> None:
+    with (
+        tarfile.open(name=archive_path, mode='w') as role_archive,
+        tempfile.TemporaryDirectory() as temp_dir_name,
+    ):
+        temp_dir_path = pathlib.Path(temp_dir_name)
+
+        meta_main_path = temp_dir_path / 'meta' / 'main.yml'
+        meta_main_path.parent.mkdir()
+        meta_main_path.write_text('')
+
+        symlink_path = temp_dir_path / 'symlink'
+        symlink_path.symlink_to(target_path)
+
+        role_archive.add(meta_main_path)
+        role_archive.add(symlink_path)
+
+        content_tarinfo = role_archive.gettarinfo(content_path, str(symlink_path))
+
+        with content_path.open('rb') as content_file:
+            role_archive.addfile(content_tarinfo, content_file)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansible-galaxy-role/tasks/dir-traversal.yml

@@ -0,0 +1,44 @@
+- name: create test directories
+  file:
+    path: '{{ remote_tmp_dir }}/dir-traversal/{{ item }}'
+    state: directory
+  loop:
+    - source
+    - target
+    - roles
+
+- name: create test content
+  copy:
+    dest: '{{ remote_tmp_dir }}/dir-traversal/source/content.txt'
+    content: |
+      some content to write
+
+- name: build dangerous dir traversal role
+  script:
+    chdir: '{{ remote_tmp_dir }}/dir-traversal/source'
+    cmd: create-role-archive.py dangerous.tar content.txt {{ remote_tmp_dir }}/dir-traversal/target/target-file-to-overwrite.txt
+    executable: '{{ ansible_playbook_python }}'
+
+- name: install dangerous role
+  command:
+    cmd: ansible-galaxy role install --roles-path '{{ remote_tmp_dir }}/dir-traversal/roles' dangerous.tar
+    chdir: '{{ remote_tmp_dir }}/dir-traversal/source'
+  ignore_errors: true
+  register: galaxy_install_dangerous
+
+- name: check for overwritten file
+  stat:
+    path: '{{ remote_tmp_dir }}/dir-traversal/target/target-file-to-overwrite.txt'
+  register: dangerous_overwrite_stat
+
+- name: get overwritten content
+  slurp:
+    path: '{{ remote_tmp_dir }}/dir-traversal/target/target-file-to-overwrite.txt'
+  register: dangerous_overwrite_content
+  when: dangerous_overwrite_stat.stat.exists
+
+- assert:
+    that:
+      - dangerous_overwrite_content.content|default('')|b64decode == ''
+      - not dangerous_overwrite_stat.stat.exists
+      - galaxy_install_dangerous is failed

test/integration/targets/ansible-galaxy-role/tasks/main.yml

@@ -59,3 +59,5 @@
 
 - name: Uninstall invalid role
   command: ansible-galaxy role remove invalid-testrole
+
+- import_tasks: dir-traversal.yml