-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Extend dotnet module #1670
base: master
Are you sure you want to change the base?
Extend dotnet module #1670
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). For more information, open the CLA check for this pull request. |
docs/modules/dotnet.rst
Outdated
|
||
.. c:type:: minor_runtime_version | ||
|
||
The major version contained in the CLI header |
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.
s/major/minor/
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.
Mostly just minor nits with this, but I very much like this PR!
docs/modules/dotnet.rst
Outdated
|
||
If CORHEADER_NATIVE_ENTRYPOINT is set, entry_point represents an RVA | ||
to a native entrypoint. If CORHEADER_NATIVE_ENTRYPOINT is not set, | ||
entry_point represents a managed entrypoint. |
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.
represents an RVA to a managed entrypoint.
This way it is clear that in both cases it is an RVA.
docs/modules/dotnet.rst
Outdated
@@ -53,12 +78,12 @@ Reference | |||
stream object has the following attributes: | |||
|
|||
.. c:member:: name | |||
|
|||
Stream name. | |||
|
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.
Nit: whitespace added here.
docs/modules/dotnet.rst
Outdated
|
||
|
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.
Nit: extra newlines here.
libyara/include/yara/dotnet.h
Outdated
// ECMA-335 Section II.23.1.10 | ||
// | ||
// These three bits contain one of the following values | ||
#define METHOD_FLAGS_MEMBER_ACCESS_MASK 0x0007 |
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.
I noticed you're adding this _MASK
definition but I'm not sure how useful it will be, as you're really just exposing it as a constant in the module without actually using it anywhere. Can we simplify a bit by removing this construct in the few places you're using it?
libyara/include/yara/dotnet.h
Outdated
// | ||
// Manifest Resource Table | ||
// ECMA-335 Section II.22.22 | ||
// |
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.
This seems to be a copy/paste mistake. :)
When CORHEADER_NATIVE_ENTRYPOINT is not set it doesn't point to an RVA. I specified this better by stating `entry_point represents a metadata token`. Finding the RVA requires parsing the metadata tables for the specified token
Co-authored-by: Wesley Shields <[email protected]>
I had the same thoughts when adding them, I was mocking up ECMA as is, but I agree that if it isn't being used, doesn't need to be present.
Co-authored-by: Wesley Shields <[email protected]>
Co-authored-by: Wesley Shields <[email protected]>
… into extend_dotnet_module
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.
updated per comments given.
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.
Apologies for the delay, this completely slipped off my radar. I just took a quick look through this and it looks good to me!
Looks good to me. The only thing I miss is adding |
Overview
This large PR is to extend the dotnet parsing capabilities to look into the .NET directory and MetaData tables further. Not all MetaData tables are being handled, just a few that have been found useful. Further table parsing could be added in the future. This will greatly extend yara capabilities combatting .NET malware that has been previously not available or requiring very complex yara rules.
Examples
Two examples are provided below, there are more in the tests and documentation. These examples take advantage of the new feature
is_dotnet
.Mixed Mode
MemberRefs with order preference (if only malware was always this easy)