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

Windows: SvcScan Binary Info #1069

Merged
merged 10 commits into from
Jan 30, 2024

Conversation

dgmcdona
Copy link
Contributor

In the original volatility framework, the svcscan plugin, in addition to gathering information about running services from the services process, also retrieved the ImagePath and ServiceDll attributes from the Windows registry: https://github.com/volatilityfoundation/volatility/blob/a438e768194a9e05eb4d9ee9338b881c0fa25937/volatility/plugins/malware/svcscan.py#L642.

This PR adds support for that in volatility3. Additionally, during testing, it became clear that the volatility3 svcscan plugin was not working correctly across all versions of Windows. Specifically, PID and Binary values were not parsing correctly in some instances, instead producing empty values. This was due to changes in the _SERVICE_RECORD structure between Windows versions, leading to incorrect offsets for the ProcessId and BinaryPath members. This PR also adds updated symbol files for the broken versions of Windows, as well as newOsDistinguisher instances for correctly detecting those Windows versions where the changes took place. I have tested these new OS versions and symbol files across all relevant versions of Windows to confirm their correctness.

Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

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

Thanks, this looks pretty good. Since the get_record_tuple was officially part of the plugin's API there's a little bit of house keeping to ensure backwards compatibility, but the rest of it looks good. I'd appreciate @iMHLv2 having a look, but if he's busy and doesn't get to it in a week or so, gimme a reminder and I'll merge it in once the housekeeping's been done. 5:)

volatility3/framework/interfaces/context.py Show resolved Hide resolved
volatility3/framework/plugins/windows/svcscan.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/windows/svcscan.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/windows/svcscan.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/windows/svcscan.py Outdated Show resolved Hide resolved
@dgmcdona
Copy link
Contributor Author

Thanks for the feedback! I think I have resolved all of the issues that you pointed out. I also retested across many samples to ensure that these fixes didn't cause any breaking changes.

Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for making the requested changes. 5:)

@ikelos
Copy link
Member

ikelos commented Jan 28, 2024

Please can you make sure the changes have been formatted using black, and then I'm happy to get this merged (the failed check will contain a diff if you don't want to install black yourself for some reason)...

Parses the binary or dll associated with each service from the Windows
SYSTEM hive and includes it in the output for each service.
There were some changes made to the service structures between certain
versions of Windows that caused the PID/binary path to fail to parse
across quite a few samples. This adds the appropriate version detection
logic and updated symbol files to ensure that services are parsed
correctly across Windows versions.
@dgmcdona dgmcdona force-pushed the dgmcdona/windows_svcscan branch 2 times, most recently from cba2d41 to 4192e8a Compare January 29, 2024 22:18
@dgmcdona
Copy link
Contributor Author

Ok, I rebased onto the latest develop branch, and reformatted using black version 24.1.1; I think it should be good to go now.

Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

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

Thanks very much, looking really good now, just a couple trivial bits to fix up and then we can merge it! 5:D

volatility3/framework/plugins/windows/svcscan.py Outdated Show resolved Hide resolved
@ikelos ikelos merged commit 2b15f3f into volatilityfoundation:develop Jan 30, 2024
14 checks passed
@ikelos
Copy link
Member

ikelos commented Jan 30, 2024

Looks good, all merged, thanks!

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.

2 participants