Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions dissect/target/plugins/os/windows/_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,14 +325,14 @@ def users(self) -> Iterator[WindowsUserRecord]:
except RegistryValueNotFoundError:
pass
else:
home = profile_image_path.value
home = self.target.resolve(profile_image_path.value)
# Use SAM username if available
name = self._sam_by_sid[sid].username if sid in self._sam_by_sid else home.split("\\")[-1]
name = self._sam_by_sid[sid].username if sid in self._sam_by_sid else home.name

yield WindowsUserRecord(
sid=subkey.name,
sid=sid,
name=name,
home=self.target.resolve(home),
home=home,
_target=self.target,
)

Expand Down
2 changes: 1 addition & 1 deletion dissect/target/plugins/os/windows/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ def machine_sid(self) -> Iterator[ComputerSidRecord]:
except (RegistryError, struct.error) as e:
self.target.log.error("Cannot read machine SID from registry") # noqa: TRY400
self.target.log.debug("", exc_info=e)
return None
return

@export(record=ComputerSidRecord)
def domain_sid(self) -> Iterator[ComputerSidRecord]:
Expand Down
50 changes: 50 additions & 0 deletions tests/plugins/os/windows/test__os.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from pathlib import PureWindowsPath
from typing import TYPE_CHECKING, Any
from unittest.mock import Mock

Expand Down Expand Up @@ -308,6 +309,55 @@ def test_windows_user_from_sam(target_win_users: Target) -> None:
assert users[1].home == windows_path("C:\\Users\\John")


def test_windows_user_registry_parsing(target_win_users: Target) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't fail if you use the previous code.

I was able to recreate the erroneous behaviour that it displays on a vm. Here i created a fake SID but it did not contain a ProfileImagePath RegistryValue. Causeing it to error and it would try to call target.resolve on home which is a None object. Shouldn't that be the behaviour this tests? / shouldn't we have a test for that specifically?

"""Verify ProfileList parsing relies on resolved path objects."""

# 1. Setup the Registry Mock Structure
sid_str = "S-1-5-21-99999-88888-77777-1001"
raw_path_str = "C:\\Users\\DuaLipa"

# Create the subkey (The user SID)
subkey_mock = Mock()
subkey_mock.name = sid_str

# Create the value for ProfileImagePath
value_mock = Mock()
value_mock.value = raw_path_str
subkey_mock.value.return_value = value_mock

# Create the registry key
key_mock = Mock()
key_mock.subkeys.return_value = [subkey_mock]

# We must overwrite the real .registry attribute with a Mock object first
target_win_users.registry = Mock()
# Now we can configure the .keys() method on that mock
target_win_users.registry.keys.return_value = [key_mock]

# 2. Setup the Resolve Mock
resolved_path = PureWindowsPath("C:/Users/DuaLipa")

# Overwrite the real .resolve method with a Mock
target_win_users.resolve = Mock(return_value=resolved_path)

# Disable SAM and Machine SID
target_win_users.sam = Mock(return_value=[])
target_win_users.machine_sid = Mock(return_value=iter([]))

# 4. Run the function
users = list(target_win_users.users())

assert len(users) == 1
record = users[0]

assert record.sid == sid_str
assert record.name == "DuaLipa"
assert record.home == resolved_path

# Verify resolve was called correctly
target_win_users.resolve.assert_called_with(raw_path_str)


@pytest.mark.parametrize(
("registry_value", "expected_hostname"),
[
Expand Down
Loading