fix: Remove conceptually unsound symlink lookup inside extfs

Symlinks can cross FS boundaries

Author: twiggler

dissect/extfs/extfs.py

@@ -39,7 +39,6 @@
 class ExtFS:
     def __init__(self, fh: BinaryIO):
         self.fh = fh
-        # self._path_cache = {}
         self._journal = None
 
         fh.seek(c_ext.EXT2_SBOFF)
@@ -123,13 +122,10 @@ def get(self, path_or_inum: str | int, node: INode | None = None) -> INode:
 
         node = node if node else self.root
         parts = path_or_inum.split("/")
-        for part_num, part in enumerate(parts):
+        for _, part in enumerate(parts):
             if not part:
                 continue
 
-            while node.filetype == stat.S_IFLNK and part_num < len(parts):
-                node = node.link_inode
-
             for entry in node.iterdir():
                 if entry.filename == part:
                     node = entry
@@ -144,13 +140,12 @@ def get_inode(
         inum: int,
         filename: str | None = None,
         filetype: int | None = None,
-        parentInum: int | None = None,
         lazy: bool = False,
     ) -> INode:
         if inum < c_ext.EXT2_BAD_INO or inum > self.sb.s_inodes_count:
             raise Error(f"inum out of range {c_ext.EXT2_BAD_INO}-{self.sb.s_inodes_count}: {inum}")
 
-        inode = INode(self, inum, filename, filetype, parentInum)
+        inode = INode(self, inum, filename, filetype)
         if not lazy:
             inode._inode = inode._read_inode()
 
@@ -186,11 +181,9 @@ def __init__(
         inum: int,
         filename: str | None = None,
         filetype: int | None = None,
-        parentInum: int | None = None,
     ):
         self.extfs = extfs
         self.inum = inum
-        self.parentInum = parentInum
         self._inode = None
 
         self.filename = filename
@@ -245,17 +238,6 @@ def link(self) -> str:
             self._link = self.open().read().decode(errors="surrogateescape")
         return self._link
 
-    @property
-    def link_inode(self) -> INode:
-        if not self._link_inum:
-            # Relative lookups work because . and .. are actual directory entries
-            link = self.link
-            relnode = None if link.startswith("/") else self.extfs.get(self.parentInum)
-            target = self.extfs.get(self.link, relnode)
-            self._link_inum = target.inum
-            return target
-        return self.extfs.get_inode(self._link_inum)
-
     @property
     def xattr(self) -> list[XAttr]:
         if not self._xattr:
@@ -364,7 +346,7 @@ def iterdir(self) -> Iterator[INode]:
                 if ftype:
                     ftype = FILETYPES[ftype]
 
-                yield self.extfs.get_inode(direntry.inode, fname, ftype, parentInum=self.inum, lazy=True)
+                yield self.extfs.get_inode(direntry.inode, fname, ftype, lazy=True)
 
             offset += direntry.rec_len
             buf.seek(offset)

tests/conftest.py

@@ -27,3 +27,7 @@ def ext4_bin() -> Iterator[BinaryIO]:
 @pytest.fixture
 def ext4_sparse_bin() -> Iterator[BinaryIO]:
     yield from gzip_file("data/ext4_sparse.bin.gz")
+
+@pytest.fixture
+def ext4_symlink_bin() -> Iterator[BinaryIO]:
+    yield from gzip_file("data/ext4_symlink_test.bin.gz")

tests/data/ext4_symlink_test.bin.gz

@@ -1,14 +1,11 @@
 from __future__ import annotations
 
 import datetime
-import gzip
 import stat
 from io import BytesIO
 from typing import TYPE_CHECKING, BinaryIO
 from unittest.mock import call, patch
 
-import pytest
-
 from dissect.extfs.c_ext import c_ext
 from dissect.extfs.extfs import EXT4, ExtFS, INode
 
@@ -80,25 +77,11 @@ def test_sparse(ext4_sparse_bin: BinaryIO) -> None:
     assert sparse_all.dataruns() == [(None, 5120)]
 
 
-@pytest.mark.parametrize(
-    "image_file",
-    [
-        ("tests/data/ext4_symlink_test1.bin.gz"),
-        ("tests/data/ext4_symlink_test2.bin.gz"),
-        ("tests/data/ext4_symlink_test3.bin.gz"),
-    ],
-)
-def test_symlinks(image_file: str) -> None:
+def test_symlinks(ext4_symlink_bin: BinaryIO) -> None:
     path = "/path/to/dir/with/file.ext"
-    expect = b"resolved!\n"
-
-    def resolve(node: INode) -> INode:
-        while node.filetype == stat.S_IFLNK:
-            node = node.link_inode
-        return node
 
-    with gzip.open(image_file, "rb") as disk:
-        assert resolve(ExtFS(disk).get(path)).open().read() == expect
+    extfs = ExtFS(ext4_symlink_bin)
+    assert extfs.get(path).link == "../../../../other/path/source/to/my/file.ext"
 
 
 @patch("dissect.extfs.extfs.INode.open", return_value=BytesIO(b"\x00" * 16))