diff --git a/tests/fixtures/bwrap-mounts/dir/file b/tests/fixtures/bwrap-mounts/dir/file new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/bwrap-mounts/dir/file @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/assemble-bad-dir-with-symlink.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/assemble-bad-dir-with-symlink.yaml new file mode 100644 index 0000000000..855151532f --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/assemble-bad-dir-with-symlink.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - role: assemble-test + src_file: dir-symlink diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/copy-bad-dir-with-symlink.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/copy-bad-dir-with-symlink.yaml new file mode 100644 index 0000000000..5a7172b1bf --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/copy-bad-dir-with-symlink.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - role: copy-test + src_file: dir-symlink diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/includevars-bad-dir-with-double-symlink.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/includevars-bad-dir-with-double-symlink.yaml new file mode 100644 index 0000000000..7127af01c9 --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/includevars-bad-dir-with-double-symlink.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - role: includevarsdir-test + src_file: dir-double-symlink diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/includevars-bad-dir-with-symlink.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/includevars-bad-dir-with-symlink.yaml new file mode 100644 index 0000000000..d771a8fe0b --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/includevars-bad-dir-with-symlink.yaml @@ -0,0 +1,4 @@ +- hosts: all + roles: + - role: includevarsdir-test + src_file: dir-symlink diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test/files/dir-symlink/one b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test/files/dir-symlink/one new file mode 120000 index 0000000000..e5576ba045 --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/assemble-test/files/dir-symlink/one @@ -0,0 +1 @@ +/opt/file \ No newline at end of file diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test/files/dir-symlink/symlink b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test/files/dir-symlink/symlink new file mode 120000 index 0000000000..22d2e57bc0 --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/copy-test/files/dir-symlink/symlink @@ -0,0 +1 @@ +/opt/dir \ No newline at end of file diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-double-symlink/double-symlink-sidekick b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-double-symlink/double-symlink-sidekick new file mode 120000 index 0000000000..7037b32406 --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-double-symlink/double-symlink-sidekick @@ -0,0 +1 @@ +../double-symlink-sidekick \ No newline at end of file diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-double-symlink/one.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-double-symlink/one.yaml new file mode 100644 index 0000000000..09189d3c2f --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-double-symlink/one.yaml @@ -0,0 +1,3 @@ +--- + +first_var: one diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-symlink/one.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-symlink/one.yaml new file mode 100644 index 0000000000..09189d3c2f --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-symlink/one.yaml @@ -0,0 +1,3 @@ +--- + +first_var: one diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-symlink/two.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-symlink/two.yaml new file mode 120000 index 0000000000..99a91abada --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir-symlink/two.yaml @@ -0,0 +1 @@ +/opt/vars.yaml \ No newline at end of file diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir/sub/recursive-link b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir/sub/recursive-link new file mode 120000 index 0000000000..a96aa0ea9d --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/dir/sub/recursive-link @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/double-symlink-sidekick/vars.yaml b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/double-symlink-sidekick/vars.yaml new file mode 120000 index 0000000000..99a91abada --- /dev/null +++ b/tests/fixtures/config/remote-action-modules/git/org_project/playbooks/roles/includevarsdir-test/vars/double-symlink-sidekick/vars.yaml @@ -0,0 +1 @@ +/opt/vars.yaml \ No newline at end of file diff --git a/tests/remote/test_remote_action_modules.py b/tests/remote/test_remote_action_modules.py index 2293883d14..5d3c27eaa9 100644 --- a/tests/remote/test_remote_action_modules.py +++ b/tests/remote/test_remote_action_modules.py @@ -83,12 +83,16 @@ class TestActionModules(AnsibleZuulTestCase): self._run_job('assemble-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE) self._run_job('assemble-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE) + self._run_job('assemble-bad-dir-with-symlink', 'FAILURE', + ERROR_ACCESS_OUTSIDE) def test_copy_module(self): self._run_job('copy-good', 'SUCCESS') self._run_job('copy-bad', 'FAILURE', ERROR_ACCESS_OUTSIDE) self._run_job('copy-bad-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE) + self._run_job('copy-bad-dir-with-symlink', 'FAILURE', + ERROR_ACCESS_OUTSIDE) def test_includevars_module(self): self._run_job('includevars-good', 'SUCCESS') @@ -100,6 +104,10 @@ class TestActionModules(AnsibleZuulTestCase): self._run_job('includevars-bad-dir', 'FAILURE', ERROR_ACCESS_OUTSIDE) self._run_job('includevars-bad-dir-symlink', 'FAILURE', ERROR_ACCESS_OUTSIDE) + self._run_job('includevars-bad-dir-with-symlink', 'FAILURE', + ERROR_ACCESS_OUTSIDE) + self._run_job('includevars-bad-dir-with-double-symlink', 'FAILURE', + ERROR_ACCESS_OUTSIDE) def test_patch_module(self): self._run_job('patch-good', 'SUCCESS') diff --git a/zuul/ansible/paths.py b/zuul/ansible/paths.py index acbcd2d18c..0584d7b929 100644 --- a/zuul/ansible/paths.py +++ b/zuul/ansible/paths.py @@ -38,15 +38,50 @@ def _full_path(path): def _is_safe_path(path, allow_trusted=False): - full_path = _full_path(path) + home_path = os.path.abspath(os.path.expanduser('~')) - if not full_path.startswith(home_path): - if allow_trusted: - trusted_path = os.path.abspath( - os.path.join(home_path, '../trusted')) - if full_path.startswith(trusted_path): + allowed_paths = [home_path] + if allow_trusted: + allowed_paths.append( + os.path.abspath(os.path.join(home_path, '../trusted'))) + + def _is_safe(path_to_check): + for allowed_path in allowed_paths: + if path_to_check.startswith(allowed_path): return True return False + + # We need to really check the whole subtree starting from path. So first + # start with the root and do an os.walk if path resolves to a directory. + full_path = _full_path(path) + if not _is_safe(full_path): + return False + + # Walk the whole tree and check dirs and files. In order to mitigate + # chained symlink attacks we also need to follow symlinks. + visited = set() + for root, dirs, files in os.walk(full_path, followlinks=True): + + # We recurse with follow links so check root first, then the files. + # The dirs will be checked during recursion. + full_root = _full_path(root) + if not _is_safe(full_root): + return False + + # NOTE: os.walk can lead to infinite recursion when following links + # so filter out the dirs for further processing if we already checked + # this one. + if full_root in visited: + del dirs[:] + # we already checked the files here so we can just continue to the + # next iteration + continue + visited.add(full_root) + + for entry in files: + full_path = _full_path(os.path.join(root, entry)) + if not _is_safe(full_path): + return False return True