151 lines
5.8 KiB
Diff
151 lines
5.8 KiB
Diff
From f3edc091523fbe301926b7a0db25fbbd96940d93 Mon Sep 17 00:00:00 2001
|
|
From: Matt Martz <matt@sivel.net>
|
|
Date: Mon, 18 Feb 2019 10:45:05 -0600
|
|
Subject: [PATCH] [stable-2.5] Disallow use of remote home directories
|
|
containing .. in their path (CVE-2019-3828) (#52133) (#52175)
|
|
|
|
* Disallow use of remote home directories containing .. in their path
|
|
|
|
* Add CVE to changelog
|
|
(cherry picked from commit b34d141)
|
|
|
|
Co-authored-by: Matt Martz <matt@sivel.net>
|
|
---
|
|
.../fragments/disallow-relative-homedir.yaml | 3 +
|
|
lib/ansible/plugins/action/__init__.py | 3 +
|
|
test/units/plugins/action/test_action.py | 61 ++++++++++++-------
|
|
3 files changed, 44 insertions(+), 23 deletions(-)
|
|
create mode 100644 changelogs/fragments/disallow-relative-homedir.yaml
|
|
|
|
diff --git a/changelogs/fragments/disallow-relative-homedir.yaml b/changelogs/fragments/disallow-relative-homedir.yaml
|
|
new file mode 100644
|
|
index 00000000000000..0ae36ef94d27fd
|
|
--- /dev/null
|
|
+++ b/changelogs/fragments/disallow-relative-homedir.yaml
|
|
@@ -0,0 +1,3 @@
|
|
+bugfixes:
|
|
+- remote home directory - Disallow use of remote home directories that include
|
|
+ relative pathing by means of `..` (CVE-2019-3828) (https://github.com/ansible/ansible/pull/52133)
|
|
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
|
|
index ba8f32e52983f8..23d95ee444bc23 100644
|
|
--- a/lib/ansible/plugins/action/__init__.py
|
|
+++ b/lib/ansible/plugins/action/__init__.py
|
|
@@ -609,6 +609,9 @@ def _remote_expand_user(self, path, sudoable=True, pathsep=None):
|
|
else:
|
|
expanded = initial_fragment
|
|
|
|
+ if '..' in os.path.dirname(expanded).split('/'):
|
|
+ raise AnsibleError("'%s' returned an invalid relative home directory path containing '..'" % self._play_context.remote_addr)
|
|
+
|
|
return expanded
|
|
|
|
def _strip_success_message(self, data):
|
|
diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py
|
|
index 52fa3bea1bece4..5221e54ed4fafb 100644
|
|
--- a/test/units/plugins/action/test_action.py
|
|
+++ b/test/units/plugins/action/test_action.py
|
|
@@ -56,6 +56,28 @@
|
|
"""
|
|
|
|
|
|
+def _action_base():
|
|
+ fake_loader = DictDataLoader({
|
|
+ })
|
|
+ mock_module_loader = MagicMock()
|
|
+ mock_shared_loader_obj = MagicMock()
|
|
+ mock_shared_loader_obj.module_loader = mock_module_loader
|
|
+ mock_connection_loader = MagicMock()
|
|
+
|
|
+ mock_shared_loader_obj.connection_loader = mock_connection_loader
|
|
+ mock_connection = MagicMock()
|
|
+
|
|
+ play_context = MagicMock()
|
|
+
|
|
+ action_base = DerivedActionBase(task=None,
|
|
+ connection=mock_connection,
|
|
+ play_context=play_context,
|
|
+ loader=fake_loader,
|
|
+ templar=None,
|
|
+ shared_loader_obj=mock_shared_loader_obj)
|
|
+ return action_base
|
|
+
|
|
+
|
|
class DerivedActionBase(ActionBase):
|
|
TRANSFERS_FILES = False
|
|
|
|
@@ -522,6 +544,18 @@ def test_action_base_sudo_only_if_user_differs(self):
|
|
finally:
|
|
C.BECOME_ALLOW_SAME_USER = become_allow_same_user
|
|
|
|
+ def test__remote_expand_user_relative_pathing(self):
|
|
+ action_base = _action_base()
|
|
+ action_base._play_context.remote_addr = 'bar'
|
|
+ action_base._low_level_execute_command = MagicMock(return_value={'stdout': b'../home/user'})
|
|
+ action_base._connection._shell.join_path.return_value = '../home/user/foo'
|
|
+ with self.assertRaises(AnsibleError) as cm:
|
|
+ action_base._remote_expand_user('~/foo')
|
|
+ self.assertEqual(
|
|
+ cm.exception.message,
|
|
+ "'bar' returned an invalid relative home directory path containing '..'"
|
|
+ )
|
|
+
|
|
|
|
class TestActionBaseCleanReturnedData(unittest.TestCase):
|
|
def test(self):
|
|
@@ -573,27 +607,8 @@ def fake_all(path_only=None):
|
|
|
|
class TestActionBaseParseReturnedData(unittest.TestCase):
|
|
|
|
- def _action_base(self):
|
|
- fake_loader = DictDataLoader({
|
|
- })
|
|
- mock_module_loader = MagicMock()
|
|
- mock_shared_loader_obj = MagicMock()
|
|
- mock_shared_loader_obj.module_loader = mock_module_loader
|
|
- mock_connection_loader = MagicMock()
|
|
-
|
|
- mock_shared_loader_obj.connection_loader = mock_connection_loader
|
|
- mock_connection = MagicMock()
|
|
-
|
|
- action_base = DerivedActionBase(task=None,
|
|
- connection=mock_connection,
|
|
- play_context=None,
|
|
- loader=fake_loader,
|
|
- templar=None,
|
|
- shared_loader_obj=mock_shared_loader_obj)
|
|
- return action_base
|
|
-
|
|
def test_fail_no_json(self):
|
|
- action_base = self._action_base()
|
|
+ action_base = _action_base()
|
|
rc = 0
|
|
stdout = 'foo\nbar\n'
|
|
err = 'oopsy'
|
|
@@ -607,7 +622,7 @@ def test_fail_no_json(self):
|
|
self.assertEqual(res['module_stderr'], err)
|
|
|
|
def test_json_empty(self):
|
|
- action_base = self._action_base()
|
|
+ action_base = _action_base()
|
|
rc = 0
|
|
stdout = '{}\n'
|
|
err = ''
|
|
@@ -621,7 +636,7 @@ def test_json_empty(self):
|
|
self.assertFalse(res)
|
|
|
|
def test_json_facts(self):
|
|
- action_base = self._action_base()
|
|
+ action_base = _action_base()
|
|
rc = 0
|
|
stdout = '{"ansible_facts": {"foo": "bar", "ansible_blip": "blip_value"}}\n'
|
|
err = ''
|
|
@@ -637,7 +652,7 @@ def test_json_facts(self):
|
|
# self.assertIsInstance(res['ansible_facts'], AnsibleUnsafe)
|
|
|
|
def test_json_facts_add_host(self):
|
|
- action_base = self._action_base()
|
|
+ action_base = _action_base()
|
|
rc = 0
|
|
stdout = '''{"ansible_facts": {"foo": "bar", "ansible_blip": "blip_value"},
|
|
"add_host": {"host_vars": {"some_key": ["whatever the add_host object is"]}
|