-
Notifications
You must be signed in to change notification settings - Fork 75
Fix duplicate mounting for virtual NTFS #1464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dissect/target/target.py
Outdated
| mounted_fs = {getattr(mnt, "ntfs", mnt) for mnt in root_fs.mounts.values()} | ||
| for fs in self.filesystems: | ||
| if fs not in root_fs.mounts.values(): | ||
| fs_obj = getattr(fs, "ntfs", fs) | ||
| if fs_obj not in mounted_fs: | ||
| # determine mount point | ||
| while root_fs.path(path).exists(): | ||
| counter += 1 | ||
| path = f"/$fs$/fs{counter}" | ||
|
|
||
| root_fs.mount(path, fs) | ||
| mounted_fs.add(fs_obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| mounted_fs = {getattr(mnt, "ntfs", mnt) for mnt in root_fs.mounts.values()} | |
| for fs in self.filesystems: | |
| if fs not in root_fs.mounts.values(): | |
| fs_obj = getattr(fs, "ntfs", fs) | |
| if fs_obj not in mounted_fs: | |
| # determine mount point | |
| while root_fs.path(path).exists(): | |
| counter += 1 | |
| path = f"/$fs$/fs{counter}" | |
| root_fs.mount(path, fs) | |
| mounted_fs.add(fs_obj) | |
| fake_ntfs = set() | |
| for fs in self.filesystems: | |
| ntfs_obj = getattr(fs, "ntfs", None) | |
| if ntfs_obj in fake_ntfs: | |
| continue | |
| if fs not in root_fs.mounts.values(): | |
| # determine mount point | |
| while root_fs.path(path).exists(): | |
| counter += 1 | |
| path = f"/$fs$/fs{counter}" | |
| root_fs.mount(path, fs) | |
| if ntfs_obj and fs.__type__ != "ntfs": | |
| # A non ntfs filesystem with a "ntfs" object means that add_virtual_ntfs_filesystem was used. | |
| # We use this ntfs object to identify the "fake" ntfs filesystem. | |
| # This functions due to the fake ntfs object is added to the filesystem after its parent. | |
| fake_ntfs.add(ntfs_obj) |
I think this would be a bit better as it only keeps a set for the "fake" ntfs objects.
This does need some changes to tests/test_target.py
diff --git a/tests/test_target.py b/tests/test_target.py
index 52a6f8a8..b7a46bb1 100644
--- a/tests/test_target.py
+++ b/tests/test_target.py
@@ -224,6 +224,7 @@ def mocked_win_volumes_fs() -> Iterator[tuple[Mock, Mock, Mock]]:
mock_good_volume.drive_letter = "W"
mock_good_fs = Mock(name="good-fs")
+ mock_good_fs.__type__ = "mock"
mock_good_fs.iter_subfs.return_value = []
def mock_filesystem_open(volume: Mock) -> Mock:
@@ -581,7 +582,9 @@ def test_empty_volume_log(target_bare: Target, caplog: pytest.LogCaptureFixture)
@pytest.mark.parametrize("nr_of_fs", [1, 2])
def test_fs_mount_others(target_unix: Target, nr_of_fs: int) -> None:
for _ in range(nr_of_fs):
- target_unix.filesystems.add(Mock())
+ fs = Mock()
+ fs.__type__ = "mock"
+ target_unix.filesystems.add(fs)
target_unix._mount_others()
@@ -595,7 +598,9 @@ def test_fs_mount_others(target_unix: Target, nr_of_fs: int) -> None:
@pytest.mark.parametrize("nr_of_fs", [1, 2])
def test_fs_mount_already_there(target_unix: Target, nr_of_fs: int) -> None:
for idx in range(nr_of_fs):
- target_unix.filesystems.add(Mock())
+ fs = Mock()
+ fs.__type__ = "mock"
+ target_unix.filesystems.add(fs)
target_unix._mount_others()
assert f"/$fs$/fs{idx}" in target_unix.fs.mountsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Miauwkeru the suggested changes are committed.
654167a to
c357246
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1464 +/- ##
=======================================
Coverage 80.75% 80.75%
=======================================
Files 396 396
Lines 34744 34750 +6
=======================================
+ Hits 28057 28063 +6
Misses 6687 6687
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
|
This PR prevents the mounting of fake virtual NTFS filesystems loaded
with
loaderutil.add_virtual_ntfs_filesystemat /$fs$/fsNInstead of comparing filesystem wrapper objects,
_mount_others()now compares the underlying NTFS object of filesystemsagainst the underlying NTFS object of already mounted filesystems.
If a filesystem does not expose an NTFS object, the filesystem itself
is used for comparison. This prevents the same (virtual) NTFS filesystem from being
mounted multiple times via different filesystem wrappers.
This change breaks test_kape.py and test_tanium.py, because those tests
were counting /$fs$/fs0 in the number of fs mounts present.
This PR fixes those tests. Those same tests can be used to test
these proposed changes.
closes #1457