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

Suspicious process creation analytics rule logic error #11215

Closed
msamoi opened this issue Oct 4, 2024 · 5 comments · Fixed by #11266
Closed

Suspicious process creation analytics rule logic error #11215

msamoi opened this issue Oct 4, 2024 · 5 comments · Fixed by #11266
Assignees

Comments

@msamoi
Copy link

msamoi commented Oct 4, 2024

The problem
The "Process Creation with Suspicious CommandLine Arguments" analytics rule contains a type error on the 3rd line of the query where an array is passed to the todynamic() function, which only accepts strings. See first screenshot.

Additionally, the operations on the 4th and 5th lines of the query expect the same variable to be both a string and dynamic type. Please look at the 2nd screenshot for clarity.

To Reproduce
Steps to reproduce the behavior:

  1. Open the rule in the analytics rule wizard
  2. Go to set rule logic
  3. Scroll down to the 3rd line of the query
  4. See error

Screenshots
1
image
2
image

@v-rusraut
Copy link
Contributor

Hi @msamoi, Thanks for flagging this issue, we will investigate this issue and get back to you with some updates. Thanks!

@v-rusraut
Copy link
Contributor

Hi @msamoi , please use the query below to check if your issue is resolved.

_ASim_ProcessEvent
| where EventType == 'ProcessCreated'
| extend CommandLineArgs = array_slice(split(CommandLine, " "), 1, -1)
| where array_length(CommandLineArgs) > 0
| mv-apply CommandLineArgs on (
where CommandLineArgs contains "base64"
)
| project
TimeGenerated,
DvcHostname,
DvcIpAddr,
DvcDomain,
TargetUsername,
TargetUsernameType,
TargetProcessName,
TargetProcessId,
CommandLine
| extend Username = iff(tostring(TargetUsernameType) == 'Windows', tostring(split(TargetUsername, '\')), TargetUsername)
| extend NTDomain = iff(tostring(TargetUsernameType) == 'Windows', tostring(split(TargetUsername, '\')), TargetUsername)
| extend Username = iff(tostring(TargetUsernameType) == 'UPN', tostring(split(TargetUsername, '@')), Username)
| extend UPNSuffix = iff(tostring(TargetUsernameType) == 'UPN', tostring(split(TargetUsername, '@')), '')

@msamoi
Copy link
Author

msamoi commented Oct 11, 2024

Hello,

Yes, the query does solve the issues described in the original post, however there are a few things I would like to point out:

  1. This part in the last few lines: (split(TargetUsername, '\') there should be two \s in the quotation marks, as currently the \ escapes the last quote and breaks the query. So, (split(TargetUsername, '\\')
  2. The query runs quite slowly. I waited 2 minutes for it to finish, but in the end I got impatient and cancelled the query instead. This query essentially checks if "base64" is present in the command line arguments. The current implementation splits the command into substrings, and checks if each substring contains the word "base64".

Isn't it faster to check each command's entire argument list at once, rather than each substring separately? I made a query based on this logic, and it finished in less than 10 seconds on the same data set. I still kept the array slicing to keep the query from matching "base64" with the command itself, instead of just the arguments:

_ASim_ProcessEvent
| where EventType == 'ProcessCreated'
| extend CommandLineArgs = strcat_array(array_slice(split(CommandLine, " "), 1, -1), " ")
| where strlen(CommandLineArgs) > 0
| where CommandLineArgs contains "base64"
| project
TimeGenerated,
DvcHostname,
DvcIpAddr,
DvcDomain,
TargetUsername,
TargetUsernameType,
TargetProcessName,
TargetProcessId,
CommandLine
| extend Username = iff(tostring(TargetUsernameType) == 'Windows', tostring(split(TargetUsername, '\\')), TargetUsername)
| extend NTDomain = iff(tostring(TargetUsernameType) == 'Windows', tostring(split(TargetUsername, '\\')), TargetUsername)
| extend Username = iff(tostring(TargetUsernameType) == 'UPN', tostring(split(TargetUsername, '@')), Username)
| extend UPNSuffix = iff(tostring(TargetUsernameType) == 'UPN', tostring(split(TargetUsername, '@')), '')

Any thoughts?

@v-rusraut
Copy link
Contributor

Hi @msamoi,we are working on updating the query as per your suggestion and will also update the solution package. We will notify you once the PR is merged.

@v-rusraut
Copy link
Contributor

Hi @msamoi , We have updated the rule and the solution package. This new update will be available in solution version 3.0.1 soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants