diff --git a/src/pylorax/creator.py b/src/pylorax/creator.py index 476577268..b59b8e7e0 100644 --- a/src/pylorax/creator.py +++ b/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 @@ -352,7 +352,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) @@ -395,7 +395,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: @@ -572,10 +572,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) diff --git a/src/pylorax/installer.py b/src/pylorax/installer.py index 655526533..0f1707e98 100644 --- a/src/pylorax/installer.py +++ b/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 @@ -356,8 +356,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: @@ -379,7 +379,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/ diff --git a/src/pylorax/ltmpl.py b/src/pylorax/ltmpl.py index 36e09e971..a752b23fb 100644 --- a/src/pylorax/ltmpl.py +++ b/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 @@ -231,9 +231,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, *pkgs): """ Return the list of files in the packages """ pkglist = [] @@ -433,6 +442,13 @@ 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): @@ -440,6 +456,9 @@ 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)) @@ -452,6 +471,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: @@ -462,6 +487,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): diff --git a/src/pylorax/sysutils.py b/src/pylorax/sysutils.py index 35f8b0fe8..981d738b2 100644 --- a/src/pylorax/sysutils.py +++ b/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("") diff --git a/src/pylorax/treebuilder.py b/src/pylorax/treebuilder.py index bc93d4f7a..5a0e31c1a 100644 --- a/src/pylorax/treebuilder.py +++ b/src/pylorax/treebuilder.py @@ -27,7 +27,7 @@ from pathlib import Path import itertools -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 @@ -61,11 +61,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.''' @@ -394,6 +398,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)) diff --git a/tests/pylorax/templates/bad-copy-cmd.tmpl b/tests/pylorax/templates/bad-copy-cmd.tmpl new file mode 100644 index 000000000..999f3f677 --- /dev/null +++ b/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 diff --git a/tests/pylorax/templates/bad-hardlink-cmd.tmpl b/tests/pylorax/templates/bad-hardlink-cmd.tmpl new file mode 100644 index 000000000..a1fe65774 --- /dev/null +++ b/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 diff --git a/tests/pylorax/templates/bad-move-cmd.tmpl b/tests/pylorax/templates/bad-move-cmd.tmpl new file mode 100644 index 000000000..d2c124515 --- /dev/null +++ b/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 diff --git a/tests/pylorax/templates/bad-move-symlink-cmd.tmpl b/tests/pylorax/templates/bad-move-symlink-cmd.tmpl new file mode 100644 index 000000000..31083cbe8 --- /dev/null +++ b/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 diff --git a/tests/pylorax/templates/bad-symlink-cmd.tmpl b/tests/pylorax/templates/bad-symlink-cmd.tmpl new file mode 100644 index 000000000..983f3bb32 --- /dev/null +++ b/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 diff --git a/tests/pylorax/test_ltmpl.py b/tests/pylorax/test_ltmpl.py index 8d1871a3b..f800bd7e5 100644 --- a/tests/pylorax/test_ltmpl.py +++ b/tests/pylorax/test_ltmpl.py @@ -229,22 +229,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") diff --git a/tests/pylorax/test_sysutils.py b/tests/pylorax/test_sysutils.py index 5edf3f4f1..cc4f148ec 100644 --- a/tests/pylorax/test_sysutils.py +++ b/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") diff --git a/tests/pylorax/test_treebuilder.py b/tests/pylorax/test_treebuilder.py index 833fcbfbe..0364073cb 100644 --- a/tests/pylorax/test_treebuilder.py +++ b/tests/pylorax/test_treebuilder.py @@ -23,7 +23,9 @@ from pylorax import ArchData, DataHolder from pylorax.dnfbase import get_dnf_base_object -from pylorax.treebuilder import RuntimeBuilder +from pylorax.executils import execWithRedirect +from pylorax.sysutils import joinpaths +from pylorax.treebuilder import RuntimeBuilder, findkernels, _write_modinfo # TODO Put these into a common test library location @contextmanager @@ -147,3 +149,64 @@ def test_skip_branding(self): branding = self.install_branding(repo_dir, skip_branding=True) self.assertEqual(branding.release, None) self.assertEqual(branding.logos, None) + + +class FindkernelsTestCase(unittest.TestCase): + def test_findkernels(self): + with tempfile.TemporaryDirectory(prefix="lorax.test.root.") as root_dir: + os.makedirs(joinpaths(root_dir, "boot")) + + # Make some fake kernel files in the temporary boot dir + for n in ["vmlinuz-7.0.0-100.fc43.x86_64", + "initramfs-7.0.0-100.fc43.x86_64.img", + "vmlinuz-7.0.1-100.fc43.x86_64", + "initramfs-7.0.0-100.fc43.x86_64.img"]: + with open(joinpaths(root_dir, "boot", n), "w", encoding="UTF-8") as f: + f.write("lorax test fake file") + + # Make a symlink to one of them that's relative + rc = execWithRedirect("/bin/ln", + ["-s", "./vmlinuz-7.0.1-100.fc43.x86_64", + joinpaths(root_dir, "boot", "vmlinuz-7.0.1-101.fc43.x86_64")]) + self.assertEqual(0, rc) + + # Make a symlink pointing outside root_dir + rc = execWithRedirect("/bin/ln", + ["-s", "../../../etc/fstab", + joinpaths(root_dir, "boot", "vmlinuz-7.0.1-102.fc43.x86_64")]) + self.assertEqual(0, rc) + + # Make an absolute symlink + rc = execWithRedirect("/bin/ln", + ["-s", "/etc/some-file", + joinpaths(root_dir, "boot", "vmlinuz-7.0.1-103.fc43.x86_64")]) + self.assertEqual(0, rc) + + # call findkernels on it + kernels = findkernels(root_dir, "boot") + kernel_versions = sorted([k.version for k in kernels]) + + # List includes kernel files plus symlink pointing inside the mount + # Does not include vmlinuz-7.0.1-102.fc43.x86_64 which points outside + self.assertEqual(["7.0.0-100.fc43.x86_64", + "7.0.1-100.fc43.x86_64", + "7.0.1-101.fc43.x86_64"], kernel_versions) + +class ModInfoTestCase(unittest.TestCase): + def test_write_modinfo(self): + modinfo = [{"name": "foo", "type": "scsi", "desc": "foo driver"}] + + with tempfile.TemporaryDirectory(prefix="lorax.test.root.") as moddir: + with open(joinpaths(moddir, "other-file"), "w", encoding="UTF-8") as f: + f.write("lorax test file") + + rc = execWithRedirect("/bin/ln", + ["-s", joinpaths(moddir, "other-file"), + joinpaths(moddir, "module-info")]) + self.assertEqual(0, rc) + + _write_modinfo(modinfo, joinpaths(moddir, "module-info")) + + # check symlinked file's content to make sure it is undisturbed + data = open(joinpaths(moddir, "other-file"), "r", encoding="UTF-8").read() + self.assertEqual("lorax test file", data)