-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix AttributeError in syscache plugin #913
Conversation
Dear @JazzCore , Thank you very much for your contribution to Dissect. |
@@ -75,7 +75,9 @@ def syscache(self): | |||
full_path = None | |||
if mft: | |||
try: | |||
full_path = self.target.fs.path("\\".join(["sysvol", mft.mft(file_segment).fullpath()])) | |||
path = mft.ntfs.mft(file_segment).full_path() |
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.
mft
already refers to a Mft
instance, so I don't think we need the round trip and can shorten to
path = mft(file_segment).full_path()
.
We can make it even more concise:
path = mft.ntfs.mft(file_segment).full_path() | |
if path := mft(file_segment).full_path(): | |
full_path = self.target.fs.path("\\".join(["sysvol", path])) |
Incidentally, when did you start experiencing this issue? I don't see any recent commits to dissect.ntfs
which might cause this. Perhaps it never worked.
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.
Looks good, thanks!
That was first time running syscache plugin for me. My guess about regression in dissect.nfts
was based on what looked like a rename in API function that was missed here when updating another parts of dissect modules. Not sure whether tests for syscache are working, maybe that's why it was missed.
Also I did some more debbuging - this happened when triaging virtualized Server 2008 that was hit with ransomware. When running this plugin it fails to get full paths from MFT records for 100+ records, no clue why. Some info from debugger for one file, there is no inode 75390
in MFT - either this is a quirk that came from MFT changes on the system, or perhaps a bug in dissect.ntfs
:
ipdb> rec = mft(file_segment)
ipdb> rec
<MftRecord 75390#902>
ipdb> dir(rec)
['__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__getstate__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_get_stream_attributes', 'attributes', 'data', 'dataruns', 'filename', 'filenames', 'from_bytes', 'from_fh', 'full_path', 'full_paths', 'get', 'has_stream', 'header', 'index', 'is_dir', 'is_file', 'is_mount_point', 'is_reparse_point', 'is_symlink', 'iterdir', 'listdir', 'ntfs', 'offset', 'open', 'reparse_point_name', 'reparse_point_record', 'reparse_point_substitute_name', 'resident', 'segment', 'size']
ipdb> rec.full_path()
ipdb> rec.filenames
<bound method MftRecord.filenames of <MftRecord 75390#902>>
ipdb> rec.filenames()
[]
ipdb> rec.data
b'FILE0\x00\x03\x00v\xed\x11A7\x00\x00\x00\x86\x03\x00\x008\x00\x01\x00\xf8\x03\x00\x00\x00\x04\x00\x00\x1d&\x01\x00\x00\x00c\x02\x01\x00\x00\x00~&\x01\x00\x06\x00!\x10\x00\x00\x00\x00\x80\x00\x00\x00\xb8\x03\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xef\r\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00\x00\x10\xf0\x14\x00\x00\x00\x00\x00\x03\xf0\x14\x00\x00\x00\x00\x00\x03\xf0\x14\x00\x00\x00\x001\x01d\'21\x03\x05\xfd\xd11\x04M%\n1\x08luo!\x10\xab(!\x10\xe3\x1e!\x10\xd2\x001\x10\xc39\xff!\x10D\x93!\x10\x99\xdc!\x101\xea!\x04\xd3\xf9!\x01\xbb\xf91\x03\xe5q#1\x04c\xd2+1\x0c\xef\x99\xf7A\x19\x97\xcfQ\x011\x0f\xba\x1c\xe8A\x10\x85\xf0\xee\xfe1\x10\xba\x16\xe1!\x10\n\xaa1\x10\xfc\xf4\xf3\x11\x10\xeb!\x10>\xd3\x11\x10\xdf!\x10\x9f\xde!\x10\r\xfd!\x10\xff\xc6!\x10P\xff\x11\x10\xdd!\x10\x89\xa31\x10\xe6I\xfe1\x10\x078\xff\x11\x10\xb1!\x10R\xff1\x10d\x9e\xf91\x10e\xfc\xfb1\x10\xaf\x82\xfb1\x10\x1bq\xfe1\x10:j\xff1\x10k-\xfe1\x10\x8a\x15\xfd!\x10\x00\xff\x11\x10\xc0!\x10 \xf9\x11\x10\xa0\x11\x10\xc0\x11\x10\xc0!\x10`\xff\x11\x10\xb0\x11\x10\x90!\x10H\xff!\x10r\xff!\x10\x86\xf8\x11\x10\xc0\x11\x10\xc0!\x10P\xff!\x10\x00\xff\x11\x10\xb0!\x10\xff!\x10\x92\xfc!\x10\xc0\xf51\x10\xe6\xd8\xfe\x11\x10\xc8\x11\x10\xb9\x11\x10\xc2!\x10W\xff!\x10u\xb0!\x10\xb0\xfa\x11\x10\xe0\x11\x10\xe0\x11\x10\xe0\x11\x10\xa0\x11\x10\xc0!\x10\x86\xf1\x11\x10\xe0\x11\x10\xe0!\x10\xce\x911\x10\xe9P\xfd!\x10\xb4\xb0!\x10\xb2\xf91\x10\xb5C\xfd\x11\x10\x8e1\x10ZH\xfb1\x10Xe\xfe1\x10\xafK\xff!\x10\xc2\xbe!\x10A\xfd!\x10\xe9\xf8!\x10\xb0\xae\x11\x10\xe0\x11\x10\xe0!\x10\x12\xfd\x11\x10\xe0!\x10\xda\xf5!\x10#\xf9\x11\x10\xe0!\x10\x85\xfd\x11\x10\xe0!\x10\xd5\xed1\x10\x9f\xe5\xfe1\x10\xd5k\xfe!\x10X\xf9\x11\x10\xaa\x11\x10\xe7\x11\x10\xd5!\x10\xdb\xb41\x10\xc03\xfb1\x10\xc1\xf9\xfe!\x10\xfd\xe4!\x10U\xbc\x11\x10\xe0!\x10.\xb4\x11\x10\xe6!\x10\x98\xc2!\x10{\xf71\x10\xf9\x83\xfe1\x10n\x7f\xfe!\x10T\x9f\x11\x10\xe0!\x10n\xfa!\x103\xef1\x10\xb2g\xff!\x10\xe5\xfc\x11\x10\xe0\x11\x10\xc0\x11\x10\xe4!\x10O\xfa!\x10\xc9\xfb!\x10/\xe2!\x10\xb2\xfe\x11\x10\xb5\x11\x10\xe0\x11\x10\xe0!\x10\xc5\xf6!\x10.\xf6\x11\x10\xe0\x11\x10\xe0!\x10\xa2\xfc!\x10R\xff!\x10\x11\xfc!\x10Y\xd1!\x10\xc8\xe2!\x10\xdb\xd7!\x10[\xd7!\x10\x12\xc6!\x10\xce\xe7!\x10#\xd9!\x10\xcc\xf2!\x10o\xf4\x11\x10\xe4!\x10\x05\xbf!\x10D\xf0!\x10\xef\xfb1\x10\xdc\x1a\xff!\x10i\xe3!\x10\xf0\xdf!\x10e\xce\x11\x10\xb5!\x10g\xd9\x11\x10\xc7!\x10\xd3\xf5!\x104\xfe!\x10\x85\xf7\x11\x10\xe0\x11\x10\xe3!\x10\x06\xe9!\x105\xcb\x11\x10\xe8!\x10\xf6\xfa!\x10\x1d\xff\x11\x10\xe6!\x10\xc7\xf3\x11\x10\xe7\x11\x10\xec!\x10$\xff\x11\x10\xac\x11\x10\xd7!\x10\x8c\x96\x11\x10\xe9!\x10\xe6\xf0\x11\x10\xeb\x11\x10\xc3!\x10\x82\xfd\x11\x10\xe8\x11\x10\xde!\x10\xbb\xe3!\x10\xf8\xd7!\x10!\x86!\x10\x11\xac\x11\x10\xe8!\x10y\xfc1\x10\xb2n\xfe!\x10y\xef\x11\x10\xa8!\x10\xe5\xfe!\x10\x89\xf0!\x10\x08\xff!\x10\xc5\xf8!\x10\xc0\xd9\x11\x10\xe6!\x10\xdb\xf0!\x10\xa3\xfb!\x10d\xf7\x11\x10\xe9!\x10[\xf5\x11\x10\xe6!\x10"\xff!\x10\xa2\xf5\x11\x10\x88!\x10T\xfa\x11\x10\xe0!\x10\xe7\xf8!\x10\x8a\xf2\x11\x10\xe8!\x10\xfd\xf6!\x10\xa7\xf6!\x10{\xfc!\x10\xd8\xa1\x11\x10\xe8!\x10;\xf8!\x10$\xe51\x10\xf6,\xfe!\x10x\xf9!\x10\xe1\x8f1\x10\t\x80\xfc\x11\x10\xe0\x11\x10\xe2!\x10v\xef\x00\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
I'm not even sure what's better - to just skip such syscache entries or return empty full_path
(since another data in syscache entry can be based on wrong MFT information)
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.
My guess about regression in dissect.nfts was based on what looked like a rename in API function that was missed here when updating another parts of dissect modules. Not sure whether tests for syscache are working, maybe that's why it was missed.
That would be correct. dissect.ntfs
was rewritten from scratch just before our open-source release, and it looks like we missed this plugin.
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.
Also I did some more debbuging - this happened when triaging virtualized Server 2008 that was hit with ransomware. When running this plugin it fails to get full paths from MFT records for 100+ records, no clue why. Some info from debugger for one file, there is no inode 75390 in MFT - either this is a quirk that came from MFT changes on the system, or perhaps a bug in dissect.ntfs:
Not sure why it cannot find the full paths for those records. Perhaps the entries got corrupted. Can you verify if the attributes are present in the raw record data?
If not, you may create a new issue on dissect.ntfs
. At least we got the AttributeError
fixed on this PR.
@@ -75,7 +75,9 @@ def syscache(self): | |||
full_path = None | |||
if mft: | |||
try: | |||
full_path = self.target.fs.path("\\".join(["sysvol", mft.mft(file_segment).fullpath()])) | |||
path = mft.ntfs.mft(file_segment).full_path() |
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.
Can you add a unit test to tests/plugins/os/windows/test_syscache.py
?
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.
Current test should cover AttributeError
caused by fullpath()
rename, but I have no clue how to add a test for second issue with MFT record problems, since this plugin gets information from MFT - there is a need to also mock broken MFT record.
Also, current test from master fails for me with PluginError
, I've tried to fix it, with no success:
$ tox -- tests/plugins/os/windows/test_syscache.py
...
collected 1 item
tests/plugins/os/windows/test_syscache.py F [100%]
======================================================================================================================================= FAILURES =======================================================================================================================================
_________________________________________________________________________________________________________________________________ test_syscache_plugin _________________________________________________________________________________________________________________________________
self = <Target /home/user/dissect.target/.tox/py3/tmp/test_syscache_plugin0/MockTarget-agy4mufo>, plugin_cls = <class 'dissect.target.plugins.os.windows.syscache.SyscachePlugin'>, check_compatible = True
def add_plugin(
self,
plugin_cls: Union[plugin.Plugin, type[plugin.Plugin]],
check_compatible: bool = True,
) -> plugin.Plugin:
"""Add and register a plugin by class.
Args:
plugin_cls: The plugin to add and register, this can either be a class or instance. When this is a class,
it will be instantiated.
check_compatible: A flag that determines if we check whether the plugin is compatible with the ``Target``.
Returns:
The ``plugin_cls`` instance.
Raises:
UnsupportedPluginError: Raised when plugins were found, but they were incompatible
PluginError: Raised when any other exception occurs while trying to load the plugin.
"""
self.log.debug("Adding plugin: %s", plugin_cls)
if not isinstance(plugin_cls, plugin.Plugin):
try:
> p = plugin_cls(self)
dissect/target/target.py:534:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
dissect/target/plugins/os/windows/syscache.py:37: in __init__
self.hive.add(regutil.RegfHive(fpath))
dissect/target/helpers/regutil.py:629: in __init__
self.hive = regf.RegistryHive(fh)
.tox/py3/lib/python3.11/site-packages/dissect/regf/regf.py:42: in __init__
self.header = c_regf.REGF_HEADER(data)
.tox/py3/lib/python3.11/site-packages/dissect/cstruct/types/structure.py:79: in __call__
return super().__call__(*args, **kwargs)
.tox/py3/lib/python3.11/site-packages/dissect/cstruct/types/base.py:47: in __call__
return cls.reads(stream)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
cls = <class 'dissect.cstruct.types.structure.REGF_HEADER'>, data = b'version https://git-lfs.github.com/spec/v1\noid sha256:b425cae888e491586ae6e9bc22e856b339af41703e374105ee711e2eacbbdff7\nsize 786432\n'
def reads(cls, data: bytes) -> BaseType:
"""Parse the given data from a bytes-like object.
Args:
data: Bytes-like object to parse.
Returns:
The parsed value of this type.
"""
> return cls._read(BytesIO(data))
E EOFError
.tox/py3/lib/python3.11/site-packages/dissect/cstruct/types/base.py:75: EOFError
The above exception was the direct cause of the following exception:
target_win = <Target /home/user/dissect.target/.tox/py3/tmp/test_syscache_plugin0/MockTarget-agy4mufo>, fs_win = <VirtualFilesystem>
def test_syscache_plugin(target_win, fs_win):
syscache_file = absolute_path("_data/plugins/os/windows/syscache/Syscache.hve")
fs_win.map_file("System Volume Information/Syscache.hve", syscache_file)
> target_win.add_plugin(SyscachePlugin)
tests/plugins/os/windows/test_syscache.py:9:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <Target /home/user/dissect.target/.tox/py3/tmp/test_syscache_plugin0/MockTarget-agy4mufo>, plugin_cls = <class 'dissect.target.plugins.os.windows.syscache.SyscachePlugin'>, check_compatible = True
def add_plugin(
self,
plugin_cls: Union[plugin.Plugin, type[plugin.Plugin]],
check_compatible: bool = True,
) -> plugin.Plugin:
"""Add and register a plugin by class.
Args:
plugin_cls: The plugin to add and register, this can either be a class or instance. When this is a class,
it will be instantiated.
check_compatible: A flag that determines if we check whether the plugin is compatible with the ``Target``.
Returns:
The ``plugin_cls`` instance.
Raises:
UnsupportedPluginError: Raised when plugins were found, but they were incompatible
PluginError: Raised when any other exception occurs while trying to load the plugin.
"""
self.log.debug("Adding plugin: %s", plugin_cls)
if not isinstance(plugin_cls, plugin.Plugin):
try:
p = plugin_cls(self)
except PluginError:
raise
except Exception as e:
> raise PluginError(f"An exception occurred while trying to initialize a plugin: {plugin_cls}", cause=e)
E dissect.target.exceptions.PluginError: An exception occurred while trying to initialize a plugin: <class 'dissect.target.plugins.os.windows.syscache.SyscachePlugin'>
dissect/target/target.py:538: PluginError
---------------------------------------------------------------------------------------------------------------------------------- Captured log setup ----------------------------------------------------------------------------------------------------------------------------------
WARNING dissect.target.target:target.py:92 <Target None>: Error loading config file: None
=============================================================================================================================== short test summary info ================================================================================================================================
FAILED tests/plugins/os/windows/test_syscache.py::test_syscache_plugin - dissect.target.exceptions.PluginError: An exception occurred while trying to initialize a plugin: <class 'dissect.target.plugins.os.windows.syscache.SyscachePlugin'>
================================================================================================================================== 1 failed in 0.26s ===================================================================================================================================
py3: exit 1 (0.54 seconds) /home/user/dissect.target> pytest --basetemp=/home/user/dissect.target/.tox/py3/tmp tests/plugins/os/windows/test_syscache.py pid=3365
py3: FAIL ✖ in 3.59 seconds
pypy3: skipped because could not find python interpreter with spec(s): pypy3
.pkg: _exit> python /home/user/.local/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta
lint: FAIL code 1 (1.56=setup[0.04]+cmd[1.52] seconds)
py3: FAIL code 1 (3.59=setup[3.05]+cmd[0.54] seconds)
pypy3: SKIP (0.25 seconds)
evaluation failed :( (5.47 seconds)
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.
You seem to be missing the LFS files in tests/_data
. See https://docs.dissect.tools/en/latest/contributing/tooling.html#testing.
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.
@JSCU-CNI Ah, my bad, missed that part, thanks! All good now.
Current test seems to not hit affected code, I'll come up with a new test later
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #913 +/- ##
==========================================
+ Coverage 77.02% 77.04% +0.01%
==========================================
Files 322 322
Lines 27582 27583 +1
==========================================
+ Hits 21244 21250 +6
+ Misses 6338 6333 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6c6949a
to
3aea665
Compare
@twiggler Added test, it should cover both exceptions. Patched two entries in |
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.
Great work!
- One small textual suggestion.
- Linter complaints about import ordering.
def test_syscache_plugin_real_mft(target_win, fs_win): | ||
filesystem = NtfsFilesystem(mft=open(absolute_path("_data/plugins/filesystem/ntfs/mft/mft.raw"), "rb")) | ||
|
||
# We need to change type of mocked filesystem, since syscache.py checks for explicit value |
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.
# We need to change type of mocked filesystem, since syscache.py checks for explicit value | |
# We need to change the type of the mocked filesystem, since syscache.py checks for explicit value |
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.
Done
3aea665
to
6b8691d
Compare
dissect.ntfs
changes probably caused regression in syscache plugin that now raises followingAttributeError
for me:Also when MFT record is missing plugin raises following
TypeError
, PR also fixes this: