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 (file)
--- 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 (file)
--- 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