Php os execution rules #206
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
Hello, These look pretty promising. I went ahead and made some inline suggestions to improve the rules you suggested. |
gfs
requested changes
May 10, 2020
Yes, agreed. I was unsure at first if the regex should also try to account for the function call, including the '('. Thank you.
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
I will also take this into account if I submit future rules. Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
Co-authored-by: Gabe Stocco <98900+gfs@users.noreply.github.com>
|
I think I accepted all proposed changes correctly, let me know if I missed something or have to accept your proposed changes in a different way. |
|
LGTM. @guyacosta I'll allow you to merge. |
gfs
approved these changes
May 10, 2020
|
@DiabloHorn Have you confirmed that the rules still work on your test data after my updates? |
|
@gfs yes, I just confirmed. Your proposed improvements work great. |
|
Thanks @DiabloHorn and @gfs |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
I'm unsure about the process of submitting rules, so I'm trying this pull request. Let me know if it has to be done in a different manner. This rules is to detect some of the PHP dynamic execution functions. I also ran the rule verification command after having made the changes.
dotnet ./ApplicationInspector.CLI.dll verifyrules -r /home/dev/tools/ApplicationInspector/AppInspector/rules/default/os/dynamic_execution.json Verify Rules command running Results succeeded Rule status Ruleid: AI034800, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI034900, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035000, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035100, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035200, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035300, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035400, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035500, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035510, Rulename: OS: Dynamic Execution, Status: True Ruleid: AI035520, Rulename: OS: Dynamic Execution, Status: True Verify Rules command completed