summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatt Martz <matt@sivel.net>2023-09-26 18:32:19 +0200
committerGitHub <noreply@github.com>2023-09-26 18:32:19 +0200
commitddf0311c63287e2d5334770377350c1e0cbfff28 (patch)
tree241ccf8ac0d0ab0b733eb0833e9f44c79f173b13
parentMisc. fixes in base.yml (#81771) (diff)
downloadansible-ddf0311c63287e2d5334770377350c1e0cbfff28.tar.xz
ansible-ddf0311c63287e2d5334770377350c1e0cbfff28.zip
Prevent roles from using symlinks to overwrite files outside of the installation directory (#81780)
* Sanitize linkname during role installs * Add tests * add clog frag
-rw-r--r--changelogs/fragments/cve-2023-5115.yml3
-rw-r--r--lib/ansible/galaxy/role.py52
-rwxr-xr-xtest/integration/targets/ansible-galaxy-role/files/create-role-archive.py45
-rw-r--r--test/integration/targets/ansible-galaxy-role/tasks/dir-traversal.yml44
-rw-r--r--test/integration/targets/ansible-galaxy-role/tasks/main.yml2
5 files changed, 123 insertions, 23 deletions
diff --git a/changelogs/fragments/cve-2023-5115.yml b/changelogs/fragments/cve-2023-5115.yml
new file mode 100644
index 0000000000..69e0ddb765
--- /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)
diff --git a/lib/ansible/galaxy/role.py b/lib/ansible/galaxy/role.py
index a71749fc13..2354ef7c3a 100644
--- a/lib/ansible/galaxy/role.py
+++ b/lib/ansible/galaxy/role.py
@@ -394,30 +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)
- 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:
- # 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 n_part == '..':
- display.warning(f"Illegal filename '{n_part}': '..' is not allowed")
+ for attr in ('name', 'linkname'):
+ attr_value = getattr(member, attr, None)
+ if not attr_value:
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)
+ 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)
+ setattr(member, attr, os.path.join(*n_final_parts))
if _check_working_data_filter():
# deprecated: description='extract fallback without filter' python_version='3.11'
diff --git a/test/integration/targets/ansible-galaxy-role/files/create-role-archive.py b/test/integration/targets/ansible-galaxy-role/files/create-role-archive.py
new file mode 100755
index 0000000000..cfd908c17b
--- /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()
diff --git a/test/integration/targets/ansible-galaxy-role/tasks/dir-traversal.yml b/test/integration/targets/ansible-galaxy-role/tasks/dir-traversal.yml
new file mode 100644
index 0000000000..c70e899879
--- /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
diff --git a/test/integration/targets/ansible-galaxy-role/tasks/main.yml b/test/integration/targets/ansible-galaxy-role/tasks/main.yml
index 3e1e6e7e71..e94176d450 100644
--- a/test/integration/targets/ansible-galaxy-role/tasks/main.yml
+++ b/test/integration/targets/ansible-galaxy-role/tasks/main.yml
@@ -64,3 +64,5 @@
- name: Uninstall invalid role
command: ansible-galaxy role remove invalid-testrole
+
+- import_tasks: dir-traversal.yml