Fix CVE-2020-1736 CVE-2020-1738

This commit is contained in:
houyingchao 2021-09-18 10:05:03 +08:00
parent bcbb8caa29
commit a27247af37
3 changed files with 198 additions and 1 deletions

121
CVE-2020-1736.patch Normal file
View File

@ -0,0 +1,121 @@
From a2ef19e48a53cc83b3a6f433013d8ff4e8f5d618 Mon Sep 17 00:00:00 2001
From: Brian Coca <brian.coca+git@gmail.com>
Date: Thu, 2 Apr 2020 11:07:51 -0400
Subject: [PATCH] stricter permissions on atomic_move when creating new file
fixes #67794
updated some tests that expected previous defaults
CVE-2020-1736
---
.../fragments/atomic_move_permissions.yml | 2 ++
lib/ansible/module_utils/common/file.py | 2 +-
.../targets/apt_repository/tasks/mode.yaml | 3 ++-
.../module_utils/basic/test_atomic_move.py | 17 ++++++++---------
4 files changed, 13 insertions(+), 11 deletions(-)
create mode 100644 changelogs/fragments/atomic_move_permissions.yml
diff --git a/changelogs/fragments/atomic_move_permissions.yml b/changelogs/fragments/atomic_move_permissions.yml
new file mode 100644
index 0000000000000..17fe58574560b
--- /dev/null
+++ b/changelogs/fragments/atomic_move_permissions.yml
@@ -0,0 +1,2 @@
+bugfixes:
+ - stricter permissions when atomic_move creates a file due to target not existing yet CVE-2020-1736
diff --git a/lib/ansible/module_utils/common/file.py b/lib/ansible/module_utils/common/file.py
index 9703ea782ebdf..3ca1253e82571 100644
--- a/lib/ansible/module_utils/common/file.py
+++ b/lib/ansible/module_utils/common/file.py
@@ -59,7 +59,7 @@
_PERM_BITS = 0o7777 # file mode permission bits
_EXEC_PERM_BITS = 0o0111 # execute permission bits
-_DEFAULT_PERM = 0o0666 # default file permission bits
+_DEFAULT_PERM = 0o0660 # default file permission bits
def is_executable(path):
diff --git a/test/integration/targets/apt_repository/tasks/mode.yaml b/test/integration/targets/apt_repository/tasks/mode.yaml
index d9895368a3fc5..0de6ca14aeae2 100644
--- a/test/integration/targets/apt_repository/tasks/mode.yaml
+++ b/test/integration/targets/apt_repository/tasks/mode.yaml
@@ -41,6 +41,7 @@
apt_repository:
repo: "{{ test_repo_spec }}"
state: present
+ mode: 0644
register: no_mode_results
- name: Gather no mode stat
@@ -127,4 +128,4 @@
# See https://github.com/ansible/ansible/issues/16370
- name: Assert mode_given_yaml_literal_600 is correct
assert:
- that: "mode_given_yaml_literal_600.stat.mode == '1130'"
\ No newline at end of file
+ that: "mode_given_yaml_literal_600.stat.mode == '1130'"
diff --git a/test/units/module_utils/basic/test_atomic_move.py b/test/units/module_utils/basic/test_atomic_move.py
index 7bd9496eddf20..58ad6e5df973e 100644
--- a/test/units/module_utils/basic/test_atomic_move.py
+++ b/test/units/module_utils/basic/test_atomic_move.py
@@ -63,7 +63,7 @@ def atomic_mocks(mocker, monkeypatch):
@pytest.fixture
def fake_stat(mocker):
stat1 = mocker.MagicMock()
- stat1.st_mode = 0o0644
+ stat1.st_mode = 0o0640
stat1.st_uid = 0
stat1.st_gid = 0
stat1.st_flags = 0
@@ -80,7 +80,8 @@ def test_new_file(atomic_am, atomic_mocks, mocker, selinux):
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', basic.DEFAULT_PERM & ~18)]
+ # 416 is what we expect with default perms set to 0640
+ assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', 416)]
if selinux:
assert atomic_am.selinux_default_context.call_args_list == [mocker.call('/path/to/dest')]
@@ -101,7 +102,7 @@ def test_existing_file(atomic_am, atomic_mocks, fake_stat, mocker, selinux):
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
+ assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', 416)]
if selinux:
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
@@ -124,10 +125,9 @@ def test_no_tty_fallback(atomic_am, atomic_mocks, fake_stat, mocker):
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
-
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
+ atomic_am.atomic_move('/path/to/src', '/path/to/dest')
@pytest.mark.parametrize('stdin', [{}], indirect=['stdin'])
@@ -152,9 +152,8 @@ def test_existing_file_stat_perms_failure(atomic_am, atomic_mocks, mocker):
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
atomic_mocks['rename'].assert_called_with(b'/path/to/src', b'/path/to/dest')
- # FIXME: Should atomic_move() set a default permission value when it cannot retrieve the
- # existing file's permissions? (Right now it's up to the calling code.
- # assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/src', basic.DEFAULT_PERM & ~18)]
+ # atomic_move() will set a default permission value when it cannot retrieve the
+ # existing file's permissions.
assert atomic_am.set_context_if_different.call_args_list == [mocker.call('/path/to/dest', mock_context, False)]
assert atomic_am.selinux_context.call_args_list == [mocker.call('/path/to/dest')]
@@ -211,7 +210,7 @@ def test_rename_perms_fail_temp_succeeds(atomic_am, atomic_mocks, fake_stat, moc
atomic_am.atomic_move('/path/to/src', '/path/to/dest')
assert atomic_mocks['rename'].call_args_list == [mocker.call(b'/path/to/src', b'/path/to/dest'),
mocker.call(b'/path/to/tempfile', b'/path/to/dest')]
- assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', basic.DEFAULT_PERM & ~18)]
+ assert atomic_mocks['chmod'].call_args_list == [mocker.call(b'/path/to/dest', 416)]
if selinux:
assert atomic_am.selinux_default_context.call_args_list == [mocker.call('/path/to/dest')]

69
CVE-2020-1738.patch Normal file
View File

@ -0,0 +1,69 @@
From b1fd71de03ae3843ac556d9b726b5f3b2441c3ed Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde <akasurde@redhat.com>
Date: Thu, 27 Feb 2020 11:42:12 +0530
Subject: [PATCH] Add whitelisting for package and service module
**security issue** (CVE-2020-1738)
When 'use' parameter is not used in package and service module,
ansible relies on ansible facts such as 'pkg_mgr' and 'service_mgr'.
This would allow arbitrary code execution on the managed node.
Fix is added by adding a whitelist of allowed package manager modules and
service manager modules to avoid arbitrary code execution on the managed node.
Fixes: #67796
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
---
changelogs/fragments/67796-package-service-fact_fix.yml | 4 ++++
lib/ansible/plugins/action/package.py | 8 ++++++++
lib/ansible/plugins/action/service.py | 5 +++++
3 files changed, 17 insertions(+)
create mode 100644 changelogs/fragments/67796-package-service-fact_fix.yml
diff --git a/changelogs/fragments/67796-package-service-fact_fix.yml b/changelogs/fragments/67796-package-service-fact_fix.yml
new file mode 100644
index 0000000000000..ce1ee71da08e0
--- /dev/null
+++ b/changelogs/fragments/67796-package-service-fact_fix.yml
@@ -0,0 +1,4 @@
+bugfixes:
+ - >
+ **security issue** Add a whitelist of modules for package and service module
+ when 'use' is not used and engine relies on pkg_mgr and service_mgr facts (CVE-2020-1738).
diff --git a/lib/ansible/plugins/action/package.py b/lib/ansible/plugins/action/package.py
index 932acccb04b66..8884086d8d6c5 100644
--- a/lib/ansible/plugins/action/package.py
+++ b/lib/ansible/plugins/action/package.py
@@ -56,6 +56,14 @@ def run(self, tmp=None, task_vars=None):
module = facts.get('ansible_facts', {}).get('ansible_pkg_mgr', 'auto')
if module != 'auto':
+ if module not in ['apk', 'apt_rpm', 'apt', 'dnf', 'homebrew_cask',
+ 'homebrew_tap', 'homebrew', 'installp', 'macports', 'mas',
+ 'openbsd_pkg', 'opkg', 'pacman', 'pkg5', 'pkgin',
+ 'pkgng', 'pkgutil', 'portage', 'portinstall', 'slackpkg',
+ 'snap', 'sorcery', 'svr4pkg', 'swdepot', 'swupd',
+ 'urpmi', 'xbps', 'yum', 'zypper']:
+ raise AnsibleActionFail('Could not find a module for package manager %s.'
+ 'Try setting the "use" option.' % module)
if module not in self._shared_loader_obj.module_loader:
raise AnsibleActionFail('Could not find a module for %s.' % module)
diff --git a/lib/ansible/plugins/action/service.py b/lib/ansible/plugins/action/service.py
index 3ebd0ae17dc90..e11ab1e287164 100644
--- a/lib/ansible/plugins/action/service.py
+++ b/lib/ansible/plugins/action/service.py
@@ -61,6 +61,11 @@ def run(self, tmp=None, task_vars=None):
module = 'service'
if module != 'auto':
+ # Check if auto detected module is valid module name or not
+ if module not in ['nosh', 'openwrt_init', 'runit',
+ 'svc', 'systemd', 'sysvinit', 'service']:
+ raise AnsibleActionFail('Could not find module for "%s" service manager. '
+ 'Try setting the "use" option.' % module)
# run the 'service' module
new_module_args = self._task.args.copy()
if 'use' in new_module_args:

View File

@ -11,7 +11,7 @@
Name: ansible
Summary: SSH-based configuration management, deployment, and task execution system
Version: 2.9.24
Release: 1
Release: 2
License: Python-2.0 and MIT and GPL+
Url: http://ansible.com
Source0: https://releases.ansible.com/ansible/%{name}-%{version}.tar.gz
@ -23,6 +23,8 @@ Requires: python3-PyYAML python3-crypto python3-paramiko python3-keyc
Requires: python3-setuptools python3-six sshpass python3-httplib2
Requires: python3-jmespath python3-jinja2
Recommends: %{name}-help = %{version}-%{release}
Patch01: CVE-2020-1736.patch
Patch02: CVE-2020-1738.patch
%description
%{common_desc}
@ -55,6 +57,8 @@ Obsoletes: %{name}-doc < %{name}-%{release}
%prep
%setup -q
%patch01 -p1
%patch02 -p1
%if 0%{?with_python3}
rm -rf %{py3dir}
cp -a . %{py3dir}
@ -114,6 +118,9 @@ cp -pr docs/docsite/rst .
%endif
%changelog
* Sat Sep 18 2021 houyingchao <houyingchao@huawei.com> - 2.9.24-2
- Fix CVE-2020-1736 CVE-2020-1738
* Fri Sep 17 2021 houyingchao <houyingchao@huawei.com> - 2.9.24-1
- update to 2.9.24