From b83c050f4755927440a517a8a7af43e9bf7fe778 Mon Sep 17 00:00:00 2001 From: Thomas Leonard Date: Mon, 28 May 2007 18:50:47 +0000 Subject: [PATCH] Tighten security when following recipes. git-svn-id: file:///home/talex/Backups/sf.net/Subversion/zero-install/trunk/0launch@1795 9f8c893c-44ee-0310-b757-c8ca8341c71e --- tests/HelloSym.tgz | Bin 0 -> 121 bytes tests/RecipeSymlink.xml | 12 ++++++++++ tests/testdownload.py | 16 +++++++++++++ zeroinstall/injector/policy.py | 2 +- zeroinstall/zerostore/unpack.py | 51 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 tests/HelloSym.tgz create mode 100644 tests/RecipeSymlink.xml diff --git a/tests/HelloSym.tgz b/tests/HelloSym.tgz new file mode 100644 index 0000000000000000000000000000000000000000..b973cc64592d0efce0610220a731e5d93ad2f799 GIT binary patch literal 121 zcwUq5=3wA74s~N-etXW4>yQBh>jfEAbq!CiGopJMix1{CI<0;1@#?|{OM>U#i~jfF z|A+I%?@y;j{x+3S+q>o3y_wOz^LKJv&h^qwmb)zxq_^~nz-a@S@~raxxtFDqZ!EL3 X-}k2OD+3ICaQ9#6w5*&#gMk46zJD{` literal 0 HcwPel00001 diff --git a/tests/RecipeSymlink.xml b/tests/RecipeSymlink.xml new file mode 100644 index 0000000..cc4ca20 --- /dev/null +++ b/tests/RecipeSymlink.xml @@ -0,0 +1,12 @@ + + + RecipeSymlink + RecipeSymlink + RecipeSymlink + + + + + + + diff --git a/tests/testdownload.py b/tests/testdownload.py index a451415..f37509c 100755 --- a/tests/testdownload.py +++ b/tests/testdownload.py @@ -145,6 +145,22 @@ class TestDownload(BaseTest): finally: sys.stdout = old_out + def testSymlink(self): + old_out = sys.stdout + try: + sys.stdout = StringIO() + self.child = server.handle_requests(('HelloWorld.tar.bz2', 'HelloSym.tgz')) + policy = autopolicy.AutoPolicy(os.path.abspath('RecipeSymlink.xml'), download_only = False) + try: + policy.download_and_execute([]) + assert False + except model.SafeException, ex: + if 'Attempt to unpack dir over symlink "HelloWorld"' not in str(ex): + raise ex + self.assertEquals(None, basedir.load_first_cache('0install.net', 'implementations', 'main')) + finally: + sys.stdout = old_out + def testAutopackage(self): old_out = sys.stdout try: diff --git a/zeroinstall/injector/policy.py b/zeroinstall/injector/policy.py index 294e16c..d2d1d77 100644 --- a/zeroinstall/injector/policy.py +++ b/zeroinstall/injector/policy.py @@ -66,7 +66,7 @@ class _Cook: try: # Unpack each of the downloaded archives into it in turn for step in self.recipe.steps: - unpack.unpack_archive(step.url, self.streams[step], tmpdir, step.extract) + unpack.unpack_archive_over(step.url, self.streams[step], tmpdir, step.extract) # Check that the result is correct and store it in the cache store.check_manifest_and_rename(self.required_digest, tmpdir) tmpdir = None diff --git a/zeroinstall/zerostore/unpack.py b/zeroinstall/zerostore/unpack.py index dbe43a5..79c30ae 100644 --- a/zeroinstall/zerostore/unpack.py +++ b/zeroinstall/zerostore/unpack.py @@ -11,7 +11,7 @@ import sha import re from logging import debug, info, warn from zeroinstall import SafeException -from zeroinstall.support import find_in_path +from zeroinstall.support import find_in_path, ro_rmtree _cpio_version = None def _get_cpio_version(): @@ -126,6 +126,55 @@ def _exec_maybe_sandboxed(writable, prog, *args): pola_args += ['-fw', writable] os.execl(_pola_run, _pola_run, *pola_args) +def unpack_archive_over(url, data, destdir, extract = None, type = None, start_offset = 0): + """Like unpack_archive, except that we unpack to a temporary directory first and + then move things over, checking that we're not following symlinks at each stage. + Use this when you want to unpack an unarchive into a directory which already has + stuff in it. + @since: 0.28""" + import stat + tmpdir = mkdtemp(dir = destdir) + try: + mtimes = [] + + unpack_archive(url, data, tmpdir, extract, type, start_offset) + + stem_len = len(tmpdir) + for root, dirs, files in os.walk(tmpdir): + relative_root = root[stem_len + 1:] or '.' + target_root = os.path.join(destdir, relative_root) + try: + info = os.lstat(target_root) + except OSError, ex: + if ex.errno != 2: + raise # Some odd error. + # Doesn't exist. OK. + os.mkdir(target_root) + else: + if stat.S_ISLNK(info.st_mode): + raise SafeException('Attempt to unpack dir over symlink "%s"!' % relative_root) + elif not stat.S_ISDIR(info.st_mode): + raise SafeException('Attempt to unpack dir over non-directory "%s"!' % relative_root) + mtimes.append((relative_root, os.lstat(os.path.join(tmpdir, root)).st_mtime)) + + for s in dirs: # Symlinks are counted as directories + src = os.path.join(tmpdir, relative_root, s) + if os.path.islink(src): + files.append(s) + + for f in files: + src = os.path.join(tmpdir, relative_root, f) + dest = os.path.join(destdir, relative_root, f) + if os.path.islink(dest): + raise SafeException('Attempt to unpack file over symlink "%s"!' % + os.path.join(relative_root, f)) + os.rename(src, dest) + + for path, mtime in mtimes[1:]: + os.utime(os.path.join(destdir, path), (mtime, mtime)) + finally: + ro_rmtree(tmpdir) + def unpack_archive(url, data, destdir, extract = None, type = None, start_offset = 0): """Unpack stream 'data' into directory 'destdir'. If extract is given, extract just that sub-directory from the archive. Works out the format from the name.""" -- 2.11.4.GIT