Description: Fix for CVE-2023-5115, based on upstream patch Author: Lee Garrett Origin: https://github.com/ansible/ansible/pull/81787 https://salsa.debian.org/python-team/packages/ansible/-/blob/debian/2.10.7+merged+base+2.10.17+dfsg-0+deb11u2/debian/patches/0010-fix-CVE-2023-5115.patch?ref_type=tags Forwarded: not-needed Applied-Upstream: https://github.com/ansible/ansible/pull/81787 Reviewed-by: Lee Garrett Last-Update: 2024-07-04 --- This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ --- /dev/null +++ b/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() --- /dev/null +++ b/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 --- /dev/null +++ b/test/integration/targets/ansible-galaxy-role/tasks/main.yml @@ -0,0 +1 @@ +- import_tasks: dir-traversal.yml --- /dev/null +++ b/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) --- a/lib/ansible/galaxy/role.py +++ b/lib/ansible/galaxy/role.py @@ -329,14 +329,36 @@ # 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) - n_archive_parent_dir = to_native(archive_parent_dir) - n_parts = n_member_name.replace(n_archive_parent_dir, "", 1).split(os.sep) - n_final_parts = [] - for n_part in n_parts: - if n_part != '..' and '~' not in n_part and '$' not in n_part: + for attr in ('name', 'linkname'): + attr_value = getattr(member, attr, None) + if not attr_value: + continue + n_attr_value = to_native(attr_value) + n_archive_parent_dir = to_native(archive_parent_dir) + n_parts = n_attr_value.replace(n_archive_parent_dir, "", 1).split(os.sep) + n_final_parts = [] + for n_part in n_parts: + # 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)) role_tar_file.extract(member, to_native(self.path)) # write out the install info file for later use --- /dev/null +++ b/test/integration/targets/ansible-galaxy-role/meta/main.yml @@ -0,0 +1 @@ +dependencies: [setup_remote_tmp_dir] --- /dev/null +++ b/test/integration/targets/setup_remote_tmp_dir/defaults/main.yml @@ -0,0 +1,2 @@ +setup_remote_tmp_dir_skip_cleanup: no +setup_remote_tmp_dir_cache_path: no --- a/test/integration/targets/setup_remote_tmp_dir/tasks/default.yml +++ b/test/integration/targets/setup_remote_tmp_dir/tasks/default.yml @@ -9,3 +9,4 @@ - name: record temporary directory set_fact: remote_tmp_dir: "{{ remote_tmp_dir.path }}" + cacheable: "{{ setup_remote_tmp_dir_cache_path | bool }}"