From 53147dd6489657d9f91a1d4d3de541f57c27d641 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Thu, 22 Feb 2018 07:46:54 +0100 Subject: [PATCH] Fix safe path checks Currently the safe path checks mostly just work on the pure given arguments. However ansible searches in several paths for the file and uses this then. Thus the safe path check can validate a non existing file (as it doesn't obey the modules search path) while the module finds the file and follows any symlinks. The solution for this is not to try to determine the correct paths ourselves but hooking into the search routine (_find_needle) the modules use for finding their local files. Using this approach we have the correct paths and can validate them properly. Change-Id: I2b3dbcfcfcf8a82e4e6df286cc3287aaa7fa2790 --- zuul/ansible/action/assemble.py | 9 ++++----- zuul/ansible/action/copy.py | 9 ++++----- zuul/ansible/action/include_vars.py | 15 +++++++++++---- zuul/ansible/action/patch.py | 9 ++++----- zuul/ansible/action/script.py | 12 ++++-------- zuul/ansible/action/template.py | 8 ++++---- zuul/ansible/action/unarchive.py | 9 ++++----- zuul/ansible/action/win_copy.py | 9 ++++----- zuul/ansible/action/win_template.py | 9 ++++----- zuul/ansible/paths.py | 15 ++++++++++++++- 10 files changed, 57 insertions(+), 47 deletions(-) diff --git a/zuul/ansible/action/assemble.py b/zuul/ansible/action/assemble.py index 30920e243e..872261eb91 100644 --- a/zuul/ansible/action/assemble.py +++ b/zuul/ansible/action/assemble.py @@ -20,14 +20,13 @@ assemble = paths._import_ansible_action_plugin("assemble") class ActionModule(assemble.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - remote_src = self._task.args.get('remote_src', False) - - if not remote_src and not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/copy.py b/zuul/ansible/action/copy.py index f00164daab..133e3b4922 100644 --- a/zuul/ansible/action/copy.py +++ b/zuul/ansible/action/copy.py @@ -20,14 +20,13 @@ copy = paths._import_ansible_action_plugin("copy") class ActionModule(copy.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - remote_src = self._task.args.get('remote_src', False) - - if not remote_src and source and not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/include_vars.py b/zuul/ansible/action/include_vars.py index 739c8a40ba..f1b047ac65 100644 --- a/zuul/ansible/action/include_vars.py +++ b/zuul/ansible/action/include_vars.py @@ -20,14 +20,21 @@ include_vars = paths._import_ansible_action_plugin("include_vars") class ActionModule(include_vars.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) source_dir = self._task.args.get('dir', None) - source_file = self._task.args.get('file', None) - for fileloc in (source_dir, source_file): - if fileloc and not paths._is_safe_path(fileloc): - return paths._fail_dict(fileloc) + # This is the handling for source_dir. The source_file is handled by + # the _find_needle override. + if source_dir: + self._set_args() + self._set_root_dir() + if not paths._is_safe_path(self.source_dir): + return paths._fail_dict(self.source_dir) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/patch.py b/zuul/ansible/action/patch.py index cac6f2fad8..f7ac842899 100644 --- a/zuul/ansible/action/patch.py +++ b/zuul/ansible/action/patch.py @@ -20,13 +20,12 @@ patch = paths._import_ansible_action_plugin("patch") class ActionModule(patch.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - remote_src = self._task.args.get('remote_src', False) - - if not remote_src and not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/script.py b/zuul/ansible/action/script.py index f67a73ef55..b726a1af6c 100644 --- a/zuul/ansible/action/script.py +++ b/zuul/ansible/action/script.py @@ -20,17 +20,13 @@ script = paths._import_ansible_action_plugin("script") class ActionModule(script.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - # the script name is the first item in the raw params, so we split it - # out now so we know the file name we need to transfer to the remote, - # and everything else is an argument to the script which we need later - # to append to the remote command - parts = self._task.args.get('_raw_params', '').strip().split() - source = parts[0] - if not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/template.py b/zuul/ansible/action/template.py index f2fbd36446..5f0e5602c0 100644 --- a/zuul/ansible/action/template.py +++ b/zuul/ansible/action/template.py @@ -20,12 +20,12 @@ template = paths._import_ansible_action_plugin("template") class ActionModule(template.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - - if not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/unarchive.py b/zuul/ansible/action/unarchive.py index 5eb3d9f7a7..f437c88717 100644 --- a/zuul/ansible/action/unarchive.py +++ b/zuul/ansible/action/unarchive.py @@ -20,13 +20,12 @@ unarchive = paths._import_ansible_action_plugin("unarchive") class ActionModule(unarchive.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - remote_src = self._task.args.get('remote_src', False) - - if not remote_src and not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/win_copy.py b/zuul/ansible/action/win_copy.py index 8fb95be368..d9dbe4dc85 100644 --- a/zuul/ansible/action/win_copy.py +++ b/zuul/ansible/action/win_copy.py @@ -20,13 +20,12 @@ win_copy = paths._import_ansible_action_plugin("win_copy") class ActionModule(win_copy.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - remote_src = self._task.args.get('remote_src', False) - - if not remote_src and not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/action/win_template.py b/zuul/ansible/action/win_template.py index 1b6a890fc7..36b475aea9 100644 --- a/zuul/ansible/action/win_template.py +++ b/zuul/ansible/action/win_template.py @@ -20,13 +20,12 @@ win_template = paths._import_ansible_action_plugin("win_template") class ActionModule(win_template.ActionModule): + def _find_needle(self, dirname, needle): + return paths._safe_find_needle( + super(ActionModule, self), dirname, needle) + def run(self, tmp=None, task_vars=None): if not paths._is_official_module(self): return paths._fail_module_dict(self._task.action) - source = self._task.args.get('src', None) - remote_src = self._task.args.get('remote_src', False) - - if not remote_src and not paths._is_safe_path(source): - return paths._fail_dict(source) return super(ActionModule, self).run(tmp, task_vars) diff --git a/zuul/ansible/paths.py b/zuul/ansible/paths.py index 05f9fdc593..dbc16561c4 100644 --- a/zuul/ansible/paths.py +++ b/zuul/ansible/paths.py @@ -22,8 +22,21 @@ import ansible.plugins.action import ansible.plugins.lookup +def _safe_find_needle(super, dirname, needle): + result = super._find_needle(dirname, needle) + if not _is_safe_path(result): + fail_dict = _fail_dict(_full_path(result)) + raise AnsibleError("{msg}. Invalid path: {path}".format( + msg=fail_dict['msg'], path=fail_dict['path'])) + return result + + +def _full_path(path): + return os.path.realpath(os.path.abspath(os.path.expanduser(path))) + + def _is_safe_path(path): - full_path = os.path.realpath(os.path.abspath(os.path.expanduser(path))) + full_path = _full_path(path) if not full_path.startswith(os.path.abspath(os.path.expanduser('~'))): return False return True