Skip to content
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

Update VelociraptorLoader based on version 0.7.0 #358

Merged
merged 14 commits into from
Aug 18, 2023

Conversation

Zawadidone
Copy link
Contributor

@Zawadidone Zawadidone commented Aug 14, 2023

Add new Velociraptor NTFS accessors

@Zawadidone
Copy link
Contributor Author

Zawadidone commented Aug 14, 2023

The volumes and paths are mapped correctly but the NTFS is not correctly parsed. I will try to find the bug that causes this.

Traceback (most recent call last):
  File "/dissect.target/dissect/target/target.py", line 404, in _load
    ldr.map(target)
  File "/dissect.target/dissect/target/loaders/velociraptor.py", line 75, in map
    map_dirs(
  File "/dissect.target/dissect/target/loaders/dir.py", line 54, in map_dirs
    loaderutil.add_virtual_ntfs_filesystem(target, dfs, **kwargs)
  File "/dissect.target/dissect/target/helpers/loaderutil.py", line 36, in add_virtual_ntfs_filesystem
    ntfs = NtfsFilesystem(boot=fh_boot, mft=fh_mft, usnjrnl=fh_usnjrnl, sds=fh_sds)
  File "/dissect.target/dissect/target/filesystems/ntfs.py", line 36, in __init__
    self.ntfs = NTFS(fh, boot=boot, mft=mft, usnjrnl=usnjrnl, sds=sds)
  File "/dissect.target/.tox/py3/lib/python3.10/site-packages/dissect/ntfs/ntfs.py", line 50, in __init__
    self.boot_sector = c_ntfs.BOOT_SECTOR(boot_fh)
  File "/dissect.target/.tox/py3/lib/python3.10/site-packages/dissect/cstruct/types/base.py", line 24, in __call__
    return self.read(*args, **kwargs)
  File "/dissect.target/.tox/py3/lib/python3.10/site-packages/dissect/cstruct/types/base.py", line 73, in read
    return self._read(obj)
  File "<compiled _BOOT_SECTOR>", line 15, in _read
EOFError

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #358 (6058b91) into main (e2d6c09) will increase coverage by 0.01%.
The diff coverage is 96.00%.

@@            Coverage Diff             @@
##             main     #358      +/-   ##
==========================================
+ Coverage   71.76%   71.77%   +0.01%     
==========================================
  Files         238      238              
  Lines       18689    18702      +13     
==========================================
+ Hits        13412    13424      +12     
- Misses       5277     5278       +1     
Flag Coverage Δ
unittests 71.77% <96.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
dissect/target/helpers/loaderutil.py 97.36% <90.00%> (-2.64%) ⬇️
dissect/target/loaders/velociraptor.py 94.73% <100.00%> (+1.18%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Schamper
Copy link
Member

Looks like it tries to open the $Boot file (which I'm not sure if Velociraptor collects?). It seems to exist (otherwise boot_fh would be None) but it's too small to actually contain a boot sector. If this is inherently a Velociraptor problem, we could put a try/except in the NTFS code that reads the boot sector.

@Zawadidone Zawadidone marked this pull request as ready for review August 15, 2023 11:32
@Zawadidone
Copy link
Contributor Author

It does collect the file but with a file size of 0 which normally is not a problem, but the bug (Velocidex/velociraptor#2892) has reintroduced itself for the second time. I don't understand the NTFS code good enough to determine if a try/except improves the loader, but if does could you add it to the PR or https://github.com/fox-it/dissect.ntfs.

I expect that the bug will be fixed in the next release of Velociraptor. I have changed the PR to ready for review because the loader functions correctly.

@Schamper
Copy link
Member

Schamper commented Aug 15, 2023

A cleaner fix might be to check for fs.stat(path).st_size > 0 in here: https://github.com/fox-it/dissect.target/blob/main/dissect/target/helpers/loaderutil.py#L44-L48. All of the files that need to be opened should be larger anyway.

When checking for stat/opening afterwards, it might be best to "optimize" that by doing a fs.get(path) first and doing those actions on the returned filesystem entry.

@Zawadidone
Copy link
Contributor Author

Okay I have added a check and now the functions work on the targets.

target-query -t * -f hostname,os  -q 
<Target VSSAnalysisAge-N> windows MSEDGEWIN10
<Target VSSAnalysisAge-Y> windows MSEDGEWIN10
<Target autoaccessor-N> windows MSEDGEWIN10
<Target autoaccessor-Y> windows MSEDGEWIN10

Nice that the NTFS parser uses default values in case there is no $Boot file https://github.com/fox-it/dissect.ntfs/blob/edb0e2de4cac0c2573b30b74c665ee71f93fc334/dissect/ntfs/ntfs.py#L72-L77.

tests/test_loaders_velociraptor.py Outdated Show resolved Hide resolved
tests/test_loaders_velociraptor.py Outdated Show resolved Hide resolved
dissect/target/helpers/loaderutil.py Outdated Show resolved Hide resolved
dissect/target/helpers/loaderutil.py Outdated Show resolved Hide resolved
dissect/target/loaders/velociraptor.py Outdated Show resolved Hide resolved
dissect/target/helpers/loaderutil.py Outdated Show resolved Hide resolved
dissect/target/helpers/loaderutil.py Outdated Show resolved Hide resolved
Copy link
Member

@Schamper Schamper left a comment

Choose a reason for hiding this comment

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

Small nitpicky comment, good to go otherwise.

dissect/target/helpers/loaderutil.py Outdated Show resolved Hide resolved
@Schamper Schamper merged commit 8e9188c into fox-it:main Aug 18, 2023
10 checks passed
@Zawadidone Zawadidone deleted the fix/velociraptor_loader branch August 18, 2023 15:54
Zawadidone added a commit to Zawadidone/dissect.target that referenced this pull request Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants