Projects
openEuler:20.03:LTS:SP3
ansible
_service:tar_scm_kernel_repo:CVE-2020-1735.patch
Sign Up
Log In
Username
Password
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File _service:tar_scm_kernel_repo:CVE-2020-1735.patch of Package ansible
From c1dedd38cb6a3ad215cf077c533c93bd978acb93 Mon Sep 17 00:00:00 2001 From: caodongxia <315816521@qq.com> Date: Tue, 2 Nov 2021 17:26:43 +0800 Subject: [PATCH] fixed fetch traversal from slurp (#68720) Reference:https://github.com/ansible/ansible/commit/290bfa820d533dc224e0c3fa7dd7c6b907ed0189.patch changelogs/fragments/fetch_no_slurp.yml | 2 + lib/ansible/plugins/action/fetch.py | 44 ++++++++----------- lib/ansible/utils/path.py | 22 ++++++++++ test/integration/targets/fetch/aliases | 1 + .../fetch/injection/avoid_slurp_return.yml | 26 +++++++++++ .../targets/fetch/injection/here.txt | 1 + .../targets/fetch/injection/library/slurp.py | 28 ++++++++++++ .../targets/fetch/run_fetch_tests.yml | 5 +++ test/integration/targets/fetch/runme.sh | 12 +++++ 9 files changed, 113 insertions(+), 26 deletions(-) create mode 100644 changelogs/fragments/fetch_no_slurp.yml create mode 100644 test/integration/targets/fetch/injection/avoid_slurp_return.yml create mode 100644 test/integration/targets/fetch/injection/here.txt create mode 100644 test/integration/targets/fetch/injection/library/slurp.py create mode 100644 test/integration/targets/fetch/run_fetch_tests.yml create mode 100644 test/integration/targets/fetch/runme.sh diff --git a/changelogs/fragments/fetch_no_slurp.yml b/changelogs/fragments/fetch_no_slurp.yml new file mode 100644 index 0000000000000..c742d40c3ba82 --- /dev/null +++ b/changelogs/fragments/fetch_no_slurp.yml @@ -0,0 +1,2 @@ +bugfixes: + - In fetch action, avoid using slurp return to set up dest, also ensure no dir traversal CVE-2019-3828. diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index fbaf992..7db5b43 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -20,13 +20,13 @@ __metaclass__ = type import os import base64 -from ansible.errors import AnsibleError +from ansible.errors import AnsibleActionFail, AnsibleActionSkip from ansible.module_utils._text import to_bytes from ansible.module_utils.six import string_types from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.utils.hashing import checksum, checksum_s, md5, secure_hash -from ansible.utils.path import makedirs_safe +from ansible.utils.path import makedirs_safe, is_subpath try: from __main__ import display @@ -47,24 +47,23 @@ class ActionModule(ActionBase): try: if self._play_context.check_mode: - result['skipped'] = True - result['msg'] = 'check mode not (yet) supported for this module' - return result + raise AnsibleActionSkip('check mode not (yet) supported for this module') source = self._task.args.get('src', None) - dest = self._task.args.get('dest', None) + original_dest = dest = self._task.args.get('dest', None) flat = boolean(self._task.args.get('flat'), strict=False) fail_on_missing = boolean(self._task.args.get('fail_on_missing', True), strict=False) validate_checksum = boolean(self._task.args.get('validate_checksum', self._task.args.get('validate_md5', True)), strict=False) + msg = '' # validate source and dest are strings FIXME: use basic.py and module specs if not isinstance(source, string_types): - result['msg'] = "Invalid type supplied for source option, it must be a string" + msg = "Invalid type supplied for source option, it must be a string" if not isinstance(dest, string_types): - result['msg'] = "Invalid type supplied for dest option, it must be a string" + msg = "Invalid type supplied for dest option, it must be a string" # validate_md5 is the deprecated way to specify validate_checksum if 'validate_md5' in self._task.args and 'validate_checksum' in self._task.args: @@ -74,11 +73,10 @@ class ActionModule(ActionBase): display.deprecated('Use validate_checksum instead of validate_md5', version='2.8') if source is None or dest is None: - result['msg'] = "src and dest are required" + msg = "src and dest are required" - if result.get('msg'): - result['failed'] = True - return result + if msg: + raise AnsibleActionFail(msg) source = self._connection._shell.join_path(source) source = self._remote_expand_user(source) @@ -106,12 +104,6 @@ class ActionModule(ActionBase): remote_data = base64.b64decode(slurpres['content']) if remote_data is not None: remote_checksum = checksum_s(remote_data) - # the source path may have been expanded on the - # target system, so we compare it here and use the - # expanded version if it's different - remote_source = slurpres.get('source') - if remote_source and remote_source != source: - source = remote_source # calculate the destination name if os.path.sep not in self._connection._shell.join_path('a', ''): @@ -120,13 +112,13 @@ class ActionModule(ActionBase): else: source_local = source - dest = os.path.expanduser(dest) + # ensure we only use file name, avoid relative paths + if not is_subpath(dest, original_dest): + # TODO: ? dest = os.path.expanduser(dest.replace(('../',''))) + raise AnsibleActionFail("Detected directory traversal, expected to be contained in '%s' but got '%s'" %(original_dest, dest)) if flat: if os.path.isdir(to_bytes(dest, errors='surrogate_or_strict')) and not dest.endswith(os.sep): - result['msg'] = "dest is an existing directory, use a trailing slash if you want to fetch src into that directory" - result['file'] = dest - result['failed'] = True - return result + raise AnsibleActionFail("dest is an existing directory, use a trailing slash if you want to fetch src into that directory") if dest.endswith(os.sep): # if the path ends with "/", we'll use the source filename as the # destination filename @@ -143,8 +135,6 @@ class ActionModule(ActionBase): target_name = self._play_context.remote_addr dest = "%s/%s/%s" % (self._loader.path_dwim(dest), target_name, source_local) - dest = dest.replace("//", "/") - if remote_checksum in ('0', '1', '2', '3', '4', '5'): result['changed'] = False result['file'] = source @@ -172,6 +162,8 @@ class ActionModule(ActionBase): result['msg'] += ", not transferring, ignored" return result + dest = os.path.normpath(dest) + # calculate checksum for the local file local_checksum = checksum(dest) @@ -188,7 +180,7 @@ class ActionModule(ActionBase): f.write(remote_data) f.close() except (IOError, OSError) as e: - raise AnsibleError("Failed to fetch the file: %s" % e) + raise AnsibleActionFail("Failed to fetch the file: %s" % e) new_checksum = secure_hash(dest) # For backwards compatibility. We'll return None on FIPS enabled systems try: diff --git a/lib/ansible/utils/path.py b/lib/ansible/utils/path.py index 717cef6..2ef1316 100644 --- a/lib/ansible/utils/path.py +++ b/lib/ansible/utils/path.py @@ -97,3 +97,25 @@ def basedir(source): dname = os.path.abspath(dname) return to_text(dname, errors='surrogate_or_strict') + +def is_subpath(child, parent): + """ + Compares paths to check if one is contained in the other + :arg: child: Path to test + :arg parent; Path to test against + """ + test = False + + abs_child = unfrackpath(child, follow=False) + abs_parent = unfrackpath(parent, follow=False) + + c = abs_child.split(os.path.sep) + p = abs_parent.split(os.path.sep) + + try: + test = c[:len(p)] == p + except IndexError: + # child is shorter than parent so cannot be subpath + pass + + return test diff --git a/test/integration/targets/fetch/aliases b/test/integration/targets/fetch/aliases index 7af8b7f..197794b 100644 --- a/test/integration/targets/fetch/aliases +++ b/test/integration/targets/fetch/aliases @@ -1 +1,2 @@ posix/ci/group2 +needs/target/setup_remote_tmp_dir diff --git a/test/integration/targets/fetch/injection/avoid_slurp_return.yml b/test/integration/targets/fetch/injection/avoid_slurp_return.yml new file mode 100644 index 0000000..c44b85e --- /dev/null +++ b/test/integration/targets/fetch/injection/avoid_slurp_return.yml @@ -0,0 +1,26 @@ +- name: ensure that 'fake slurp' does not poison fetch source + hosts: localhost + gather_facts: False + tasks: + - name: fetch with relative source path + fetch: src=../injection/here.txt dest={{output_dir}} + become: true + register: islurp + + - name: fetch with normal source path + fetch: src=here.txt dest={{output_dir}} + become: true + register: islurp2 + + - name: ensure all is good in hollywood + assert: + that: + - "'..' not in islurp['dest']" + - "'..' not in islurp2['dest']" + - "'foo' not in islurp['dest']" + - "'foo' not in islurp2['dest']" + + - name: try to trip dest anyways + fetch: src=../injection/here.txt dest={{output_dir}} + become: true + register: islurp2 diff --git a/test/integration/targets/fetch/injection/here.txt b/test/integration/targets/fetch/injection/here.txt new file mode 100644 index 0000000..493021b --- /dev/null +++ b/test/integration/targets/fetch/injection/here.txt @@ -0,0 +1 @@ +this is a test file diff --git a/test/integration/targets/fetch/injection/library/slurp.py b/test/integration/targets/fetch/injection/library/slurp.py new file mode 100644 index 0000000..3916ae8 --- /dev/null +++ b/test/integration/targets/fetch/injection/library/slurp.py @@ -0,0 +1,28 @@ +#!/usr/bin/python +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +DOCUMENTATION = """ + module: fakeslurp + short_desciptoin: fake slurp module + description: + - this is a fake slurp module + options: + _notreal: + description: really not a real slurp + author: + - me +""" + +import json +import random +bad_responses = ['../foo', '../../foo', '../../../foo', '/../../../foo', '/../foo', '//..//foo', '..//..//foo'] + + +def main(): + print(json.dumps(dict(changed=False, content='', encoding='base64', source=random.choice(bad_responses)))) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/fetch/run_fetch_tests.yml b/test/integration/targets/fetch/run_fetch_tests.yml new file mode 100644 index 0000000..f2ff1df --- /dev/null +++ b/test/integration/targets/fetch/run_fetch_tests.yml @@ -0,0 +1,5 @@ +- name: call fetch_tests role + hosts: testhost + gather_facts: false + roles: + - fetch_tests diff --git a/test/integration/targets/fetch/runme.sh b/test/integration/targets/fetch/runme.sh new file mode 100644 index 0000000..3d5b75c --- /dev/null +++ b/test/integration/targets/fetch/runme.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +set -eux + +# setup required roles +ln -s ../../setup_remote_tmp_dir roles/setup_remote_tmp_dir + +# run old type role tests +ansible-playbook -i ../../inventory run_fetch_tests.yml -e "output_dir=${OUTPUT_DIR}" -v "$@" + +# run tests to avoid path injection from slurp when fetch uses become +ansible-playbook -i ../../inventory injection/avoid_slurp_return.yml -e "output_dir=${OUTPUT_DIR}" -v "$@" -- 2.27.0
Locations
Projects
Search
Status Monitor
Help
Open Build Service
OBS Manuals
API Documentation
OBS Portal
Reporting a Bug
Contact
Mailing List
Forums
Chat (IRC)
Twitter
Open Build Service (OBS)
is an
openSUSE project
.