From: Philipp Gesang Date: Fri, 4 Nov 2016 16:00:59 +0000 (+0100) Subject: add unit test for overwriting symlinks X-Git-Tag: v2.2~8^2~3 X-Git-Url: http://developer.intra2net.com/git/?a=commitdiff_plain;h=43ce978b75b788049422850acf72a1e9b75921d0;p=python-delta-tar add unit test for overwriting symlinks Currently, we implement the behavior of GNU Tar: Subsequent files in an archive override previous ones, which is also true of symlinks. --- diff --git a/deltatar/deltatar.py b/deltatar/deltatar.py index 8bbff51..f889d3e 100644 --- a/deltatar/deltatar.py +++ b/deltatar/deltatar.py @@ -1452,12 +1452,12 @@ class RestoreHelper(object): except OSError: self._deltatar.logger.warning \ ("Not restoring symlink %s from tarball: placeholder " - "file was deleted during extraction") + "file was deleted during extraction" % fullpath) continue if st.st_dev != st_dev or st.st_ino != st_ino: self._deltatar.logger.warning \ ("Not restoring symlink %s from tarball: placeholder " - "file was modified during extraction") + "file was modified during extraction" % fullpath) continue # at this point we’re certain we’re dealing with the placeholder we # created so we can remove it and create the actual symlink diff --git a/testing/test_deltatar.py b/testing/test_deltatar.py index a31dc51..2c3ceea 100644 --- a/testing/test_deltatar.py +++ b/testing/test_deltatar.py @@ -1494,6 +1494,66 @@ class DeltaTarTest(BaseTest): " a valid symlink!" % (str(source), str(os.stat(fullpath)))) + def test_restore_malicious_symlinks(self): + ''' + Creates a full backup containing a symlink and a file of the same name. + This simulates a symlink attack with a link pointing to some external + path that is abused to write outside the extraction prefix. + ''' + + deltatar = DeltaTar(mode=self.MODE, password=self.PASSWORD, + logger=self.consoleLogger) + + # create first backup + deltatar.create_full_backup(source_path="source_dir", + backup_path="backup_dir") + + assert os.path.exists("backup_dir") + shutil.rmtree("source_dir") + + tar_filename = deltatar.volume_name_func('backup_dir', True, 0) + tar_path = os.path.join("backup_dir", tar_filename) + + # add symlinks to existing archive + + def add_symlink (a, name, dst): + l = tarfile.TarInfo("snapshot://%s" % name) + l.type = tarfile.SYMTYPE + l.linkname = dst + a.addfile(l) + + def add_file (a, name): + f = tarfile.TarInfo("snapshot://%s" % name) + f.type = tarfile.REGTYPE + a.addfile(f) + return a + + testpath = "symlinks/pernicious-link" + testdst = "symlinks/pernicious-link" + with tarfile.open(tar_path,mode="w") as a: + add_symlink(a, testpath, testdst) + add_file(a, testpath) + + dstinfo = \ + { "exists": os.path.exists(testdst) + , "chksum": os.path.isfile(testdst) and self.md5sum(testdst) or None } + + deltatar.restore_backup(target_path="source_dir", + backup_tar_path=tar_path) + + # check whether we got the link or the file + fullpath = os.path.join("source_dir", testpath) + # since tarfile unconditionally opens files with the flag combo + # “O_WRONLY | O_CREAT | O_TRUNC” (like GNU tar without the -k flag) we + # expect the link to be present, not the file from the archive; also, + #the link target may not contain the contents of the dummy file + assert os.path.islink(fullpath) + dst = os.readlink(fullpath) + assert os.path.exists(dst) == dstinfo["exists"] + chksum = dstinfo.get("chksum") + if chksum is not None: + assert self.md5sum(dst) == chksum + class DeltaTar2Test(DeltaTarTest): ''' Same as DeltaTar but with specific ":" mode