diff options
author | Matt Martz <matt@sivel.net> | 2019-02-13 10:38:28 -0600 |
---|---|---|
committer | Toshio Kuratomi <a.badger@gmail.com> | 2019-02-13 09:24:28 -0800 |
commit | 4be3215d2f9f84ca283895879f0c6ce1ed7dd333 (patch) | |
tree | 7b6e49537f7b1c9682ff665c9c968fdc7348e755 | |
parent | b5122be9e19a14694247c6de3789db9478b567f8 (diff) | |
download | ansible-4be3215d2f9f84ca283895879f0c6ce1ed7dd333.tar.gz |
[stable-2.6] Disallow use of remote home directories containing .. in their path (CVE-2019-3828) (#52133)
* 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>
-rw-r--r-- | changelogs/fragments/disallow-relative-homedir.yaml | 3 | ||||
-rw-r--r-- | lib/ansible/plugins/action/__init__.py | 3 | ||||
-rw-r--r-- | test/units/plugins/action/test_action.py | 61 |
3 files changed, 44 insertions, 23 deletions
diff --git a/changelogs/fragments/disallow-relative-homedir.yaml b/changelogs/fragments/disallow-relative-homedir.yaml new file mode 100644 index 0000000000..0ae36ef94d --- /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 9bc27931eb..e0ca825d69 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -628,6 +628,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): 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 04f287561f..f14ad5c4ba 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -56,6 +56,28 @@ WINDOWS_ARGS = "<<INCLUDE_ANSIBLE_MODULE_JSON_ARGS>>" """ +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 @@ -526,6 +548,18 @@ class TestActionBase(unittest.TestCase): 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): @@ -577,27 +611,8 @@ class TestActionBaseCleanReturnedData(unittest.TestCase): 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' @@ -611,7 +626,7 @@ class TestActionBaseParseReturnedData(unittest.TestCase): 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 = '' @@ -625,7 +640,7 @@ class TestActionBaseParseReturnedData(unittest.TestCase): 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 = '' @@ -641,7 +656,7 @@ class TestActionBaseParseReturnedData(unittest.TestCase): # 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"]} |