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

extend rule features and rename #969

Merged
merged 3 commits into from
Dec 3, 2024
Merged

extend rule features and rename #969

merged 3 commits into from
Dec 3, 2024

Conversation

mr-tz
Copy link
Collaborator

@mr-tz mr-tz commented Dec 2, 2024

closes #966

@mr-tz
Copy link
Collaborator Author

mr-tz commented Dec 2, 2024

@Ezbeat @jtothej please take a look

@Ezbeat
Copy link

Ezbeat commented Dec 2, 2024

Thank you for taking my feedback into consideration.

  1. Regarding the ProcessSignaturePolicy configuration, wouldn't it be clearer and more consistent to use policy.MicrosoftSignedOnly instead of policy.flags? This would enhance readability and align with the explicit naming convention used in other mitigation policies.

  2. For the detection of blockdlls, rather than relying on the InitializeProcThreadAttributeList function, how about using the UpdateProcThreadAttribute function? This approach could directly focus on when the Attribute is actually set, potentially improving accuracy and reducing false positives.

@mr-tz
Copy link
Collaborator Author

mr-tz commented Dec 2, 2024

nice, great feedback here, thanks! does the rule now look good to you?

@Ezbeat
Copy link

Ezbeat commented Dec 2, 2024

Looks good. Thank you for considering the suggestions!

Additionally, after reviewing the rules, it seems that the blockdlls condition overlaps with the rule defined in:

protect-spawned-processes-with-mitigation-policies.yml

What are your thoughts on this? If there is indeed an overlap, we could consider removing the blockdlls condition from this rule to avoid redundancy.

@mr-tz
Copy link
Collaborator Author

mr-tz commented Dec 3, 2024

Oh 🤦 @jtothej already did all the work... Thanks, for noticing. I'll update accordingly.

@mr-tz mr-tz merged commit 1adcf13 into master Dec 3, 2024
3 checks passed
@mr-tz mr-tz deleted the fix/966 branch December 3, 2024 13:12
@mr-tz
Copy link
Collaborator Author

mr-tz commented Dec 3, 2024

Thanks, @Ezbeat (and @jtothej)!!

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.

Modify ProcessDynamicCodePolicy Value
2 participants