-
Notifications
You must be signed in to change notification settings - Fork 567
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
add dynamic analysis #1697
add dynamic analysis #1697
Conversation
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 read through all 3,600 lines of changes. i didn't see any major architectural issues - that's great!
i added small items, suggested changes, and TODOs (via checklist items) inline here. please check and resolve so that we can track outstanding work here. major efforts ive added to the original PR comment text.
there are a lot of small things to tweak, though, perhaps two dozen things of 30 mins each to think about and implement.
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 read through all 3,600 lines of changes. i didn't see any major architectural issues - that's great!
i added small items, suggested changes, and TODOs (via checklist items) inline here. please check and resolve so that we can track outstanding work here. major efforts ive added to the original PR comment text.
there are a lot of small things to tweak, though, perhaps two dozen things of 30 mins each to think about and implement.
This comment was marked as resolved.
This comment was marked as resolved.
since the primary operation is `contain()`, set is more appropriate than tuple.
make a local copy of the scopes dict
im worried this will interact poorly with our rule cache, unless we add more handling there, which needs more testing. so, since the filtering likely has only a small impact on performance, revert the rule filtering changes for simplicity.
we should also see what the experience is like against a ransomware report with a million file read/writes. |
fix whitespace removal in format check
verbose: show process name and other human-level details
Fix global features and display
only check and display file limitation once
set os, arch, format in meta table
…nt/capa into dynamic-feature-extraction
* README: adapt for dynamic capa * README.md: fix duplication error * Update README.md Co-authored-by: Moritz <mr-tz@users.noreply.github.com> * documentation: add review suggestions * documentation: newline fix * Update README.md Co-authored-by: Moritz <mr-tz@users.noreply.github.com> * Update README.md Co-authored-by: Moritz <mr-tz@users.noreply.github.com> * Update README.md Co-authored-by: Moritz <mr-tz@users.noreply.github.com> --------- Co-authored-by: Moritz <mr-tz@users.noreply.github.com> Co-authored-by: Willi Ballenthin <willi.ballenthin@gmail.com>
…nt/capa into dynamic-feature-extraction
This is a draft PR so that we can begin to review the whole set of changes that @yelhamer proposes to enable dynamic analysis in capa. I'm not sure if we'll merge from this PR or re-create one later that we keep a bit tidier.
For the time being, when a change needs a review, lets continue to open PRs against the branch dynamic-feature-extraction, not update this PR. Its ok to merge small "suggested changes" from here when little discussion is required.
TODO:
CAPE traced APIs vs. rule APIs #1843-> to be addressed laterdon't load signatures for dynamic analysis-> converted to issue dynamic: don't load signatures for dynamic analysis #1875