Backport symlink fixes to f43-branch

src/pylorax/creator.py

@@ -44,7 +44,7 @@
 from pylorax.installer import novirt_install, virt_install, InstallError
 from pylorax.treebuilder import TreeBuilder, RuntimeBuilder
 from pylorax.treebuilder import findkernels
-from pylorax.sysutils import joinpaths, remove
+from pylorax.sysutils import joinpaths, remove, safe_joinpaths
 
 
 # Default parameters for rebuilding initramfs, override with --dracut-arg or --dracut-conf
@@ -362,7 +362,7 @@ def make_livecd(opts, mount_dir, work_dir):
     # I think these should be release specific, not from lorax, but for now
     configdir = joinpaths(opts.lorax_templates,"live/config_files/")
     configdir_path = "tmp/config_files"
-    fullpath = joinpaths(mount_dir, configdir_path)
+    fullpath = safe_joinpaths(mount_dir, configdir_path)
     if os.path.exists(fullpath):
         remove(fullpath)
     copytree(configdir, fullpath)
@@ -405,7 +405,7 @@ def mount_boot_part_over_root(img_mount):
             mount("/dev/mapper/"+dev, mnt=tmp_mount_dir)
             if is_boot_part(tmp_mount_dir):
                 umount(tmp_mount_dir)
-                sysroot_boot_dir = joinpaths(root_dir, "boot")
+                sysroot_boot_dir = safe_joinpaths(root_dir, "boot")
                 mount("/dev/mapper/"+dev, mnt=sysroot_boot_dir)
                 break
             else:
@@ -590,10 +590,10 @@ def make_live_images(opts, work_dir, disk_img):
     log.info("Rebuilding initramfs for live")
     with Mount(rootfs_img, opts="loop") as mnt_dir:
         try:
-            mount(joinpaths(mnt_dir, "boot"), opts="bind", mnt=joinpaths(mnt_dir, sys_root, "boot"))
-            rebuild_initrds_for_live(opts, joinpaths(mnt_dir, sys_root), work_dir)
+            mount(joinpaths(mnt_dir, "boot"), opts="bind", mnt=safe_joinpaths(mnt_dir, sys_root, "boot"))
+            rebuild_initrds_for_live(opts, safe_joinpaths(mnt_dir, sys_root), work_dir)
         finally:
-            umount(joinpaths(mnt_dir, sys_root, "boot"), delete=False)
+            umount(safe_joinpaths(mnt_dir, sys_root, "boot"), delete=False)
 
     remove(squashfs_root_dir)
 

src/pylorax/installer.py

@@ -33,7 +33,7 @@
 from pylorax.imgutils import mkqemu_img, mktar, mkcpio, mkfsimage_from_disk
 from pylorax.monitor import LogMonitor
 from pylorax.mount import IsoMountpoint
-from pylorax.sysutils import joinpaths
+from pylorax.sysutils import joinpaths, remove
 from pylorax.treebuilder import udev_escape
 
 
@@ -422,8 +422,8 @@ def novirt_install(opts, disk_img, disk_size, cancel_func=None, tar_img=None):
 
     # Clean up /tmp/ from previous runs to prevent stale info from being used
     for path in ["/tmp/yum.repos.d/", "/tmp/yum.cache/"]:
-        if os.path.isdir(path):
-            shutil.rmtree(path)
+        if os.path.exists(path):
+            remove(path)
 
     args = ["--kickstart", opts.ks[0], "--cmdline"]
     if opts.anaconda_args:
@@ -445,7 +445,7 @@ def novirt_install(opts, disk_img, disk_size, cancel_func=None, tar_img=None):
     elif opts.make_tar or opts.make_oci:
         # Install under dirinstall_path, make sure it starts clean
         if os.path.exists(dirinstall_path):
-            shutil.rmtree(dirinstall_path)
+            remove(dirinstall_path)
 
         if opts.make_oci:
             # OCI installs under /rootfs/

src/pylorax/ltmpl.py

@@ -28,7 +28,7 @@
 from subprocess import CalledProcessError
 import shutil
 
-from pylorax.sysutils import joinpaths, cpfile, mvfile, replace, remove
+from pylorax.sysutils import joinpaths, safe_joinpaths, cpfile, mvfile, replace, remove
 from pylorax.dnfhelper import LoraxDownloadCallback, LoraxRpmCallback
 from pylorax.base import DataHolder
 from pylorax.executils import runcmd, runcmd_output
@@ -413,9 +413,18 @@ def __init__(self, inroot, outroot, dbo=None, fatalerrors=True,
 
     def _out(self, path):
         return joinpaths(self.outroot, path)
+
     def _in(self, path):
         return joinpaths(self.inroot, path)
 
+    def _insideout(self, path):
+        """ Return true if path is really inside outroot """
+        try:
+            _ = safe_joinpaths(self.outroot, path)
+        except RuntimeError:
+            return False
+        return True
+
     def _filelist(self, *pkg_specs):
         """ Return the list of files in the packages matching the globs """
         # libdnf5's filter_installed query will not work unless the base it reset and reloaded.
@@ -671,13 +680,23 @@ def hardlink(self, src, dest):
         '''
         if isdir(self._out(dest)):
             dest = joinpaths(dest, basename(src))
+
+        if not self._insideout(src):
+            raise RuntimeError(f"paths outside outroot not allowed on {src}")
+
+        if not self._insideout(dest):
+            raise RuntimeError(f"paths outside outroot not allowed on {dest}")
+
         os.link(self._out(src), self._out(dest))
 
     def symlink(self, target, dest):
         '''
         symlink SRC DEST
           Create a symlink at DEST which points to SRC.
         '''
+        if not self._insideout(dest):
+            raise RuntimeError(f"paths outside outroot not allowed on {dest}")
+
         if rexists(self._out(dest)):
             self.remove(dest)
         os.symlink(target, self._out(dest))
@@ -690,6 +709,12 @@ def copy(self, src, dest):
           If DEST doesn't exist, SRC will be copied to a file with
           that name, if the path leading to it exists.
         '''
+        if not self._insideout(src):
+            raise RuntimeError(f"paths outside outroot not allowed on {src}")
+
+        if not self._insideout(dest):
+            raise RuntimeError(f"paths outside outroot not allowed on {dest}")
+
         try:
             cpfile(self._out(src), self._out(dest))
         except shutil.Error as e:
@@ -700,6 +725,12 @@ def move(self, src, dest):
         move SRC DEST
           Move SRC to DEST.
         '''
+        if not self._insideout(src):
+            raise RuntimeError(f"paths outside outroot not allowed on {src}")
+
+        if not self._insideout(dest):
+            raise RuntimeError(f"paths outside outroot not allowed on {dest}")
+
         mvfile(self._out(src), self._out(dest))
 
     def remove(self, *fileglobs):

src/pylorax/sysutils.py

@@ -44,6 +44,18 @@ def joinpaths(*args, **kwargs):
         return path
 
 
+def safe_joinpaths(*args, **kwargs):
+    """ safe_joinpaths joins paths and checks for an escape
+    and raises RuntimeError if the final path is not under the first.
+
+    NOTE: The path must exist in order to be properly evaluated
+    """
+    path = joinpaths(*args, follow_symlinks=True)
+    if not path.startswith(args[0]):
+        raise RuntimeError(f"path does not start with {args[0]}")
+    return path
+
+
 def touch(fname):
     with open(fname, "w") as f:
         f.write("")

src/pylorax/treebuilder.py

@@ -28,7 +28,7 @@
 import libdnf5 as dnf5
 from libdnf5.common import QueryCmp_EQ as EQ
 
-from pylorax.sysutils import joinpaths, remove
+from pylorax.sysutils import joinpaths, safe_joinpaths, remove
 from pylorax.base import DataHolder
 from pylorax.ltmpl import LoraxTemplateRunner
 import pylorax.imgutils as imgutils
@@ -58,11 +58,15 @@ def read_module_set(name):
                 (name, _ext) = os.path.splitext(mod) # foo.ko -> (foo, .ko)
                 desc = module_desc(joinpaths(root,mod)) or "%s driver" % name
                 modinfo.append(dict(name=name, type=modtype, desc=desc))
+    _write_modinfo(modinfo, outfile or joinpaths(moddir,"module-info"))
 
-    out = open(outfile or joinpaths(moddir,"module-info"), "w")
-    out.write("Version 0\n")
-    for mod in sorted(modinfo, key=lambda m: m.get('name')):
-        out.write('{name}\n\t{type}\n\t"{desc:.65}"\n'.format(**mod))
+def _write_modinfo(modinfo, outfile):
+    if os.path.islink(outfile):
+        os.unlink(outfile)
+    with open(outfile, "w", encoding="UTF-8") as out:
+        out.write("Version 0\n")
+        for mod in sorted(modinfo, key=lambda m: m.get('name')):
+            out.write('{name}\n\t{type}\n\t"{desc:.65}"\n'.format(**mod))
 
 class RuntimeBuilder(object):
     '''Builds the anaconda runtime image.
@@ -417,6 +421,11 @@ def findkernels(root="/", kdir="boot"):
     kernels = []
     bootfiles = os.listdir(joinpaths(root, kdir))
     for f in bootfiles:
+        # Exclude files that are symlinks pointing outside of root
+        try:
+            _ = safe_joinpaths(root, kdir, f)
+        except RuntimeError:
+            continue
         match = kre.match(f)
         if match:
             kernel = DataHolder(path=joinpaths(kdir, f))

tests/pylorax/templates/bad-copy-cmd.tmpl

@@ -0,0 +1,3 @@
+<%page />
+append /lorax-file "A text file"
+copy /lorax-file ../../var/tmp/copied-file

tests/pylorax/templates/bad-hardlink-cmd.tmpl

@@ -0,0 +1,3 @@
+<%page />
+append /lorax-file "A hardlinked file"
+hardlink /lorax-file ../../var/tmp/linked-file

tests/pylorax/templates/bad-move-cmd.tmpl

@@ -0,0 +1,3 @@
+<%page />
+append /lorax-file "A text file"
+move /lorax-file ../../var/tmp/moved-file

tests/pylorax/templates/bad-move-symlink-cmd.tmpl

@@ -0,0 +1,4 @@
+<%page />
+append /lorax-file "A text file"
+symlink /var/tmp/linked-file /symlinked-file-2
+move /lorax-file /symlinked-file-2

tests/pylorax/templates/bad-symlink-cmd.tmpl

@@ -0,0 +1,3 @@
+<%page />
+append /lorax-file "A symlinked file"
+symlink /lorax-file ../../var/tmp/existing-file

tests/pylorax/test_ltmpl.py

@@ -321,22 +321,45 @@ def test_hardlink(self):
         self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/linked-file")))
         self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/lorax-dir/lorax-file")))
 
+    def test_bad_hardlink(self):
+        """Test a hardlink template command pointing outside the outroot"""
+        with self.assertRaises(RuntimeError):
+            self.runner.run("bad-hardlink-cmd.tmpl")
+
     def test_symlink(self):
         """Test symlink template command"""
         self.runner.run("symlink-cmd.tmpl")
         self.assertTrue(os.path.islink(joinpaths(self.root_dir, "/symlinked-file")))
 
+    def test_bad_symlink(self):
+        """Test symlink template command pointing outside the outroot"""
+        with self.assertRaises(RuntimeError):
+            self.runner.run("bad-symlink-cmd.tmpl")
+
     def test_copy(self):
         """Test copy template command"""
         self.runner.run("copy-cmd.tmpl")
         self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/copied-file")))
 
+    def test_bad_copy(self):
+        """Test copy template command pointing outside the outroot"""
+        with self.assertRaises(RuntimeError):
+            self.runner.run("bad-copy-cmd.tmpl")
+
     def test_move(self):
         """Test move template command"""
         self.runner.run("move-cmd.tmpl")
         self.assertFalse(os.path.exists(joinpaths(self.root_dir, "/lorax-file")))
         self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/moved-file")))
 
+    def test_bad_move(self):
+        """Test move template command pointing outside the outroot"""
+        with self.assertRaises(RuntimeError):
+            self.runner.run("bad-move-cmd.tmpl")
+
+        with self.assertRaises(RuntimeError):
+            self.runner.run("bad-move-symlink-cmd.tmpl")
+
     def test_remove(self):
         """Test remove template command"""
         self.runner.run("remove-cmd.tmpl")

tests/pylorax/test_sysutils.py

@@ -19,8 +19,9 @@
 import tempfile
 import os
 
+from pylorax.executils import execWithRedirect
 from pylorax.sysutils import joinpaths, touch, replace, chown_, chmod_, remove, linktree
-from pylorax.sysutils import _read_file_end
+from pylorax.sysutils import safe_joinpaths, _read_file_end
 
 class SysUtilsTest(unittest.TestCase):
     def test_joinpaths(self):
@@ -34,6 +35,26 @@ def test_joinpaths(self):
             self.assertEqual(joinpaths(tdname, "link-file", follow_symlinks=True),
                              os.path.join(tdname, "real-file"))
 
+    def test_safe_joinpaths(self):
+        with tempfile.TemporaryDirectory() as tdname:
+            self.assertEqual(safe_joinpaths(tdname, "foo", "bar", "baz"), tdname+"/foo/bar/baz")
+
+            with open(os.path.join(tdname, "real-file"), "w") as f:
+                f.write("lorax test file")
+            os.symlink(os.path.join(tdname, "real-file"), os.path.join(tdname, "link-file"))
+
+            self.assertEqual(safe_joinpaths(tdname, "link-file"), os.path.join(tdname, "real-file"))
+
+    def test_safe_joinpaths_bad(self):
+        with tempfile.TemporaryDirectory() as tdname:
+            os.symlink("/var/tmp/some-file", os.path.join(tdname, "link-file"))
+
+            with self.assertRaises(RuntimeError):
+                _ = safe_joinpaths(tdname, "link-file")
+
+            with self.assertRaises(RuntimeError):
+                _ = safe_joinpaths(tdname, "../../", "link-file")
+
     def test_touch(self):
         touch_file="/var/tmp/lorax-test-touch-file"
         touch(touch_file)
@@ -69,6 +90,20 @@ def test_remove(self):
         remove(remove_file)
         self.assertFalse(os.path.exists(remove_file))
 
+    def test_remove_symlink_dir(self):
+        # Make sure remove doesn't removed symlinked directories
+        with tempfile.TemporaryDirectory() as tdname:
+            test_file = os.path.join(tdname, "lorax-test-file")
+            with open(test_file, "w", encoding="UTF-8") as f:
+                f.write("test was here")
+            rc = execWithRedirect("/bin/ln", ["-s", tdname, "/var/tmp/lorax-test-link"])
+            self.assertEqual(0, rc)
+
+            remove("/var/tmp/lorax-test-link")
+
+            self.assertTrue(os.path.exists(tdname))
+            self.assertFalse(os.path.exists("/var/tmp/lorax-test-link"))
+
     def test_linktree(self):
         with tempfile.TemporaryDirectory() as tdname:
             path = os.path.join("one", "two", "three")