[stable-2.7] Disallow use of remote home directories containing .. in their path (CVE-2019-3828) (#52133) (#52173)

* 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>
This commit is contained in:
Matt Martz 2019-02-13 11:23:06 -06:00 committed by Toshio Kuratomi
parent 7918323832
commit 396a2f7471
3 changed files with 44 additions and 23 deletions

View file

@ -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)

View file

@ -614,6 +614,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):

View file

@ -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"]}