Yamato-Security / hayabusa-rules

Curated Windows event log Sigma rules used in Hayabusa and Velociraptor.
Other
144 stars 23 forks source link

Incomplete field conversion in `process_creation` rules #443

Closed fukusuket closed 1 year ago

fukusuket commented 1 year ago

Describe the bug process_creation rules are automatically converted to EventID: 4688 (Security) and EventID: 1 (Sysmon) rules. However, due to imperfect field conversion, rules are created that cannot be detected.

Currently, logsource_mapping.py does only the following field conversions. https://github.com/Yamato-Security/hayabusa-rules/blob/65703b21168dd11a99b5f0ce640f612cb5fd63c0/tools/sigmac/windows-audit.yaml#L43-L48

Actual behavior Undetectable rules are created. For example, the rule below uses field ParentCommandLine,IntegrityLevel that does not exist in EventID: 4688 (Security) https://github.com/Yamato-Security/hayabusa-rules/blob/65703b21168dd11a99b5f0ce640f612cb5fd63c0/sigma/builtin/process_creation/proc_creation_win_uac_bypass_cleanmgr.yml#L19-L28

Expected behavior Undetectable rules are not created.

Additional context The field check script in the Sigma repository lists the following fields:

Channel: Security, EventID: 4688
"SubjectUserSid", 
"SubjectUserName", 
"SubjectDomainName", 
"SubjectLogonId", 
"NewProcessId", 
"NewProcessName", 
"TokenElevationType", 
"ProcessId", 
"CommandLine", 
"TargetUserSid", 
"TargetUserName", 
"TargetDomainName", 
"TargetLogonId", 
"ParentProcessName", 
"MandatoryLabel"
Channel: Microsoft-Windows-Sysmon/Operational, EventID: 1
"RuleName",
"UtcTime",
"ProcessGuid", 
"ProcessId", 
"Image", 
"FileVersion", 
"Description", 
"Product", 
"Company", 
"OriginalFileName", 
"CommandLine", 
"CurrentDirectory", 
"User", 
"LogonGuid", 
"LogonId", 
"TerminalSessionId", 
"IntegrityLevel", 
"Hashes", 
"ParentProcessGuid", 
"ParentProcessId", 
"ParentImage", 
"ParentCommandLine", 
"ParentUser"
YamatoSecurity commented 1 year ago

@fukusuket This is a good issue! If the process creation rule is looking for fields that don't exist in security 4688 then we shouldn't create that rule as it will just slow things down. There is also a small chance that someone could create a rule that looks for a field in Security 4688 but does not exist in Sysmon 1 so we should probably check for that and not create a sysmon 1 rule in that case.

fukusuket commented 1 year ago

Thank you for your comment :) Yes, it also improves performance if unnecessary rules are not created! I will try to implement the check function💪

fukusuket commented 1 year ago

I implemented the check and it looks like the 3811 - 3321 = 490 rule has incomplete field.

main

[INFO:logsource_mapping.py:376] [3811] files created successfully. Created files were saved under [./converted_sigma_rules].
[INFO:logsource_mapping.py:377] Script took [10.26] seconds.

445

[INFO:logsource_mapping.py:410] [3321] files created successfully. Created files were saved under [./converted_sigma_rules].
[INFO:logsource_mapping.py:411] Script took [9.70] seconds.

I tried investigating the fields of the incomplete rule... There were 395 incomplete rules containing field OriginalFileName (with Sec:4688)

...
[WARNING:logsource_mapping.py:196] This rule has incompatible field.{'process_creation': {'EventID': 4688, 'Channel': 'Security'}, 'selection_img': [{'NewProcessName|endswith': '\\cmd.exe'}, {'OriginalFileName': 'Cmd.Exe'}], 'selection_parent': {'ParentProcessName|endswith': '\\sqlservr.exe'}, 'condition': 'all of selection_*'}. skip conversion.
...
fukusuke@fukusukenoAir Desktop % cat warn.txt | grep OriginalFileName | wc -l
     395
YamatoSecurity commented 1 year ago

@fukusuket

Sec4688 and Sysmon1 difference

For the logic, if a process_creation rule has one of the following fields, we should not make a sysmon 1 rule:

SubjectUserSid
TokenElevationType
TargetUserSid
TargetUserName
TargetDomainName
TargetLogonId

(The red ones in the picture)

If a process_creation rule has one of the following fields, we should not make a security 4688 rule:

RuleName
UtcTime
ProcessGuid
FileVersion
Description
Product
Company
OriginalFileName
CurrentDirectory
LogonGuid
TerminalSessionId
Hashes
ParentProcessGuid
ParentCommandLine
ParentUser

Image gets mapped to NewProcessName, ParentImage to ParentProcessName and LogonId to SubjectLogonId.

ProcessId and ParentProcessId can be mapped to NewProcessId and ProcessId, however, we need to convert the decimal PID to hex.

IntegrityLevel maps to MandatoryLabel but the following conversion needs to be done in the data portion: Low -> S-1-16-4096 Medium -> S-1-16-8192 High -> S-1-16-12288 System -> S-1-16-16384

Note: sometimes in sigma rules, it will be written System but other times SYSTEM so we need to check the integrity level case in-sensitive.

What do you think?

fukusuket commented 1 year ago

@YamatoSecurity I implemented https://github.com/Yamato-Security/hayabusa-rules/issues/443#issuecomment-1610810550 :) https://github.com/Yamato-Security/hayabusa-rules/pull/445/commits/5d386c91fa46a9bc9f84cde162c93d30e1bc194c , then I have a question!

When checking the https://github.com/Yamato-Security/hayabusa-rules/pull/445/commits/5d386c91fa46a9bc9f84cde162c93d30e1bc194c results , I noticed that.. In the new impl, the following rule has an OriginalFileName, so it will be treated as an error in the Security:4688 rule, https://github.com/Yamato-Security/hayabusa-rules/blob/1a88a2e2b0af819c0f2772d27166d1837b54daa5/sigma/builtin/process_creation/proc_creation_win_powershell_non_interactive_execution.yml#L18-L27 However, since it is an OR condition, it is a useful rule when detecting with NewProcessName ... 🤔 Should these rules be treated as correct rules if there is at least one correct field in the OR condition?

YamatoSecurity commented 1 year ago

@fukusuket Thanks alot! That is an interesting edge case that I did not think of. Yes, if there is an OR condition like this and one contains a valid field in 4688 (like NewProcessName) then we should create a 4688 rule as well.

fukusuket commented 1 year ago

@YamatoSecurity Thank you for your comment :) I'll add a new check logic for OR conditions! !💪