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

Feat/warn for dynamic dotnet #2568

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

v1bh475u
Copy link

closes #1864

Checklist

  • No CHANGELOG update needed
  • No new tests needed
  • No documentation update needed

CHANGELOG.md Outdated Show resolved Hide resolved
capa/main.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

I've added some comments inline. Would you review and let me know what you think @v1bh475u? If the proposals work for you, please try them out. If you disagree, let's figure out alternatives.

@v1bh475u
Copy link
Author

@williballenthin I have made the changes as we had discussed. Please review them whenever you are free.

Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

see inline suggestions.

CHANGELOG.md Outdated Show resolved Hide resolved
capa/capabilities/common.py Outdated Show resolved Hide resolved
capa/capabilities/common.py Outdated Show resolved Hide resolved
capa/capabilities/common.py Outdated Show resolved Hide resolved
Comment on lines -751 to +756
def find_file_limitations_from_cli(args, rules: RuleSet, file_extractors: list[FeatureExtractor]) -> bool:
def find_static_limitations_from_cli(args, rules: RuleSet, file_extractors: list[FeatureExtractor]) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

one thing we're losing in the changes is the documentation/naming that makes clear that the static limitations are derived from file extractors (like pefile) not full disassembly-derived extractors (like vivisect). they should be lightweight. so lets find the right place to re-add this documentation.

Copy link
Author

Choose a reason for hiding this comment

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

Can you please give some more insights?

capa/main.py Outdated Show resolved Hide resolved
capa/main.py Outdated
args:
args: The parsed command line arguments from `install_common_args`.

Handles dynamic dotnet samples.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Handles dynamic dotnet samples.
For example, notifies when handling .NET samples in a sandbox, which may rely on different API patterns.

Copy link
Collaborator

Choose a reason for hiding this comment

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

please put explanations before the args/returns/raises blocks.

capa/main.py Outdated Show resolved Hide resolved
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.

dynamic: warn user for .NET samples
2 participants