From a45497384093efe7bcee4251de162935c63f31da Mon Sep 17 00:00:00 2001
From: Matt Martz <matt@sivel.net>
Date: Tue, 12 Feb 2019 11:25:33 -0600
Subject: [PATCH] Disallow use of remote home directories containing .. in
 their path

---
 .../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

Modified by Bruno Cornec for Mageia

diff --git a/changelogs/fragments/disallow-relative-homedir.yaml b/changelogs/fragments/disallow-relative-homedir.yaml
new file mode 100644
index 0000000000000..37ab4ba09d9d8
--- /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 `..`
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 1b3929a3a74c7..cba47fb0e70cb 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -559,6 +559,8 @@ def _remote_expand_user(self, path, sudoable=True, pathsep=None):
         if len(split_path) > 1:
             return self._connection._shell.join_path(initial_fragment, *split_path[1:])
         else:
+            if '..' in os.path.dirname(initial_fragment).split('/'):
+                raise AnsibleError("'%s' returned an invalid relative home directory path containing '..'" % self._play_context.remote_addr)
             return initial_fragment
 
     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 89d7377e8d054..79e4456a277d6 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
 
@ -552,6 +564,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):
@@ -577,27 +611,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'
@@ -611,7 +626,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 = ''
@@ -625,7 +640,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 = ''
@@ -641,7 +656,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"]}