cmueller/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Disallow use of remote home directories containing .. in their path (CVE-2019-3828) (#52133)

Browse source 
* Disallow use of remote home directories containing .. in their path

* Add CVE to changelog
This commit is contained in:
Matt Martz 2019-02-13 10:38:28 -06:00 committed by GitHub
parent 9f081ca04f
commit b34d141eed
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 44 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
changelogs/fragments/disallow-relative-homedir.yaml Normal file
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)

3
lib/ansible/plugins/action/__init__.py
View file

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

61
test/units/plugins/action/test_action.py
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):
action_base._low_level_execute_command('ECHO SAME', sudoable=True)
become.build_become_command.assert_called_once_with("ECHO SAME", shell)
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"]}