Add yara-timeout setting for in-monitor scans#121
Add yara-timeout setting for in-monitor scans#121ncatlin wants to merge 2 commits intokevoreilly:capemonfrom
Conversation
Summary of ChangesHello @ncatlin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the flexibility of CAPE's in-monitor YARA scanning by making the scan timeout configurable. Previously, a fixed 1-second timeout could lead to incomplete analysis and missing data when YARA rules were complex or extensive. The change allows users to adjust this timeout, preventing silent failures and ensuring more comprehensive behavioral analysis and memory dumps, thereby improving the reliability and diagnostic capabilities of the system. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable yara-timeout setting for in-monitor YARA scans, replacing the previous hardcoded 1-second timeout. This is a valuable addition, as it provides more flexibility for users with complex YARA rules that may require longer scan times. The changes in config.h, config.c, and YaraHarness.c correctly implement this new configuration option by adding the yara_timeout field to the global configuration, parsing it from the config file, setting a default value, and using it in the YaraScan function. My review includes one suggestion to improve the robustness of the configuration parsing.
| else if (!stricmp(key, "yara-timeout")) { | ||
| g_config.yara_timeout = (int)strtol(value, NULL, 10); | ||
| if (g_config.yara_timeout == 0) | ||
| DebugOutput("In-monitor YARA scan timeout set to unlimited.\n"); | ||
| else | ||
| DebugOutput("In-monitor YARA scan timeout set to %d seconds.\n", g_config.yara_timeout); | ||
| } |
There was a problem hiding this comment.
The strtol function is used to parse the yara-timeout value, but there's no error handling. If an invalid string (e.g., "abc") is provided, strtol will return 0, which will be interpreted as an unlimited timeout. This could lead to unexpected behavior if the user makes a typo in the configuration. It's better to validate the input to ensure it's a valid non-negative integer and log an error for invalid values.
else if (!stricmp(key, "yara-timeout")) {
char *endptr;
long timeout = strtol(value, &endptr, 10);
if (endptr == value || *endptr != '\0' || timeout < 0) {
DebugOutput("Invalid value for yara-timeout: '%s'. Must be a non-negative integer.\n", value);
} else {
g_config.yara_timeout = (int)timeout;
if (g_config.yara_timeout == 0)
DebugOutput("In-monitor YARA scan timeout set to unlimited.\n");
else
DebugOutput("In-monitor YARA scan timeout set to %d seconds.\n", g_config.yara_timeout);
}
}There was a problem hiding this comment.
Error handling was in my first attempt and happy to re-add it, there just isn't any error handling in the rest of the configuration parsing and I didn't want to add clutter - so removed it to fit in with all the other strtol parsing.
Invalid values will result in a Debug log being emitted, correctly showing the timeout is disabled - unless the user passes a negative value for some reason, which will also result there being no timeout.
|
Hi, thank you for this PR, it looks great. I will review and merge it tomorrow... ... but I am too intrigued to wait until then to ask how, on your first time using cape, you have ended up with monitor yara sigs that are pushing the scan timeout - what yara sigs have you added and what do they do?! |
|
It's a good question - i've tried to replicate it with some profiling and think I was misremembering a bit. So I did a few test scans with 30-40 rules from the base and community repos, and while most were <1ms duration, there were a few >100ms scans So if you have more than a few dozen rules and you are scanning regions >10 mb, you will almost certainly hit timeouts and I'm confident this feature is still useful to have. The real problem though (and now I remember why it was hard to debug) was this: I can't replicate this using yara.exe to scan dumped memory, but from what I can tell of the YARA source code, this happens when it fails on warnings - possibly because a rule has a string matching 60k+ times. https://github.com/CAPESandbox/community/blob/master/data/yara/binaries/indicator_tools.yar causes this even when it's the only rule file. I guess the lesson here is to be very careful with passing only the bare minimum of rules for runtime scanning, and do your bulk YARA work on downloaded memory dumps. |
|
Yes you hit on exactly where I was going with my question, as it sounds like you cannot be using the monitor yaras for what they are intended for, which is not detection in the classic sense. Their only reason for existing is to allow options in the cape_options metadata to be applied dynamically. If you are adding sigs with no cape_options metadata then you are definitely wasting your time! |
|
I still like the PR tho 🙂 |
2 participants
Hi, my first experience of using CAPE was spending hours trying to debug why no behavioural information or memory dumps were being reported. It turned out that I had been overambitious with YARA rules and the memory scanner was breaching its timeout (and not logging it) but there was no way to configure this hardcoded 1-second duration without completely recompiling and replacing capemon.dll.
Getting tired of doing that - so this PR adds a simple 'yara-timeout' monitor option that overrides the default - if supplied - otherwise there is no effect on behaviour.