rabbitstack / fibratus

Adversary tradecraft detection, protection, and hunting
https://www.fibratus.io
Other
2.21k stars 189 forks source link

feat(yara): Adding yara scan trigger for PAGE_EXECUTE_READWRITE allocations #324

Closed TheAwakener closed 1 month ago

TheAwakener commented 1 month ago

Adding yara scan trigger for PAGE_EXECUTE_READWRITE allocations, useful when PE image does not contain malicious code, for example, when shellcode is downloaded or decrypted after execution

rabbitstack commented 1 month ago

@TheAwakener thanks for your contribution. I'll get back to your PR shortly.

rabbitstack commented 1 month ago

@TheAwakener General question. If the scan is initiated on every RWX memory allocation event, what are the expected performance penalties? I image we could face a significant event rate and overwhelm the Yara scanner.

TheAwakener commented 1 month ago

@rabbitstack RWX is a rare and suspicious event for memory allocations, for example, dll images are loaded in a mapped and RX region. If you allocate heap for file reading it's default permission is RW. If you want we can make and additional check for regions unmapped with RWX.

rabbitstack commented 1 month ago

@TheAwakener that sounds good to me. Please, take a look at the other comments. Once they are addressed, the PR should be good to merge.

Bonus request: if you could write a test to cover this new signal, that would be awesome.

TheAwakener commented 1 month ago

@rabbitstack that's would be greate, although I'm new to contributions, can you tell me please where are the other comments to be addressed?

rabbitstack commented 1 month ago

@TheAwakener Go to your PR and scroll down a bit in the Conversation tab. You'll see comments/suggestions I left on individual code lines. Fix them locally, then commit and push.

TheAwakener commented 1 month ago

@rabbitstack ok, maybe I'm missing something but can't see comments on the code:

Screenshot_20240915-100419

rabbitstack commented 1 month ago

@TheAwakener take a look at the screenshot below. The conversation tab has a badge indicating the interactions on the PR. Lo

Screenshot_20240915-195724~2 Screenshot_20240915-195730

TheAwakener commented 1 month ago

@rabbitstack I already added a commit with the test case for VirtualAlloc with RWX protection flags, although I haven't been able to see the review comments you mention, they simply do not appear in the comments flow:

image

rabbitstack commented 1 month ago

@TheAwakener can you see the comments now?

TheAwakener commented 1 month ago

@rabbitstack yeah, thank you

TheAwakener commented 1 month ago

@rabbitstack new commit pushed with the requested changes

rabbitstack commented 1 month ago

I think the test should be more realistic involving VirtualAlloc followed by WriteProcessMemory planting a trivial payload to match the rule. But we can leave the improvement for later. I'll merge the PR once mandatory checks pass. Congrats for your first contribution 🎉

TheAwakener commented 1 month ago

@rabbitstack thank you for allow me to participate in this wonderful project