bradleyjkemp / sigma-go Goto Github PK

I hit an issue today with this rule:

https://github.com/SigmaHQ/sigma/blob/master/rules/windows/process_creation/proc_creation_win_susp_non_exe_image.yml

Because one of the matchers is :

    filter_optional_null:
        Image: null

This matcher hits the 'default' case in this switch statement:

func (rule *RuleEvaluator) getMatcherValues(ctx context.Context, matcher sigma.FieldMatcher) ([]string, error) {
	matcherValues := []string{}
	for _, abstractValue := range matcher.Values {
		value := ""

		switch abstractValue := abstractValue.(type) {
		case string:
			value = abstractValue
		case int, float32, float64, bool:
			value = fmt.Sprintf("%v", abstractValue)
		default:
			return nil, fmt.Errorf("expected scalar field matching value got: %v (%T)", abstractValue, abstractValue)
		}

Here's my code:

			eventmap := match.Event
			ctx := ctx.Background()
			result, err := rule_evaluator.Matches(ctx, eventmap)
			if err != nil {
				fmt.Printf("** ERROR evaluating match with ID [%v], err %v\n", match.MatchId, err)
...

Here's what the error returned from rule_evaluator.Matches:

** ERROR evaluating match with ID [30], err error evaluating search filter_optional_null: expected scalar field matching value got: <nil> (<nil>)

Hey mate, super appreciative of this library, I've just discovered it fails to parse some rules from the official Sigma repo, e.g. this one. It erorrs out with cannot unmarshal !!map into string.

I am super new to SIGMA rules, but digging into it, the issue could possibly be that looking at the spec can be lists of maps. E.g. this basic rules fails to be parsed by sigma-go but according the spec it is valid (with the CommandLine being logical OR'd together):

title: Test that doesn't work
id: e0b0c2ab-3d52-46d9-8cb7-049dc775fbd1
status: experimental
description: A Sigma rule that fails to parse using sigma-go
author: pathtofile
date: 2022/01/23
logsource:
    category: process_creation
    product: windows
level: high
detection:
    baddetect:
        - CommandLine|contains: 'apple'
        - CommandLine|contains: 'bannana'
    condition: baddetect

I'm using sigma-go along with the public Sigma process_creation rules and found several that segfault like this:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1025aca1c]

goroutine 1 [running]:
github.com/bradleyjkemp/sigma-go/evaluator.(*RuleEvaluator).getMatcherValues(0x140001ed3f8, {0x102639b78, 0x14000116000}, {{0x1400012c318, 0xb}, {0x1400013bf30, 0x1, 0x1}, {0x1400013bf40, 0x2, ...}})
	/Users/jness/v/onprem/pkg/mod/github.com/bradleyjkemp/[email protected]/evaluator/evaluate_search.go:141 +0x1fc
github.com/bradleyjkemp/sigma-go/evaluator.RuleEvaluator.evaluateSearch({{{0x1400012c2e8, 0x13}, {{0x14000117760, 0x10}, {0x14000117740, 0x7}, {0x0, 0x0}, {0x0, 0x0}}, ...}, ...}, ...)
	/Users/jness/v/onprem/pkg/mod/github.com/bradleyjkemp/[email protected]/evaluator/evaluate_search.go:121 +0x184
github.com/bradleyjkemp/sigma-go/evaluator.RuleEvaluator.Matches({{{0x1400012c2e8, 0x13}, {{0x14000117760, 0x10}, {0x14000117740, 0x7}, {0x0, 0x0}, {0x0, 0x0}}, ...}, ...}, ...)
	/Users/jness/v/onprem/pkg/mod/github.com/bradleyjkemp/[email protected]/evaluator/evaluate.go:102 +0x148
main.main()
	/Users/jness/v/onprem/src/veramine.com/cmd/util/sigmatest/main.go:105 +0x4f8

These two rules in particular:

https://github.com/SigmaHQ/sigma/blob/master/rules/windows/process_creation/proc_creation_win_bitsadmin_download_susp_targetfolder.yml
https://github.com/SigmaHQ/sigma/blob/master/rules/windows/process_creation/proc_creation_win_control_panel_item.yml

Notice that both these rules are looking for fields with a %. When I remove those %s, it no longer segfaults.

    CommandLine|contains:
      - '\System32\'
      - '%System%'

        CommandLine|contains: 
            - 'C:\Users\Public\'
            - '%public%'
            - '\Desktop\'

I'm curious on your thoughts here. I am using this repo, but mainly just for the rule parsing. I have separate evaluation logic for our use internally. This logic would be improved with the ability to retain the type information from Sigma searches in YAML format. Currently, all values (i.e. float, string, integer) are loaded as strings. If the value is not scalar, it is empty when loaded by sigma-go.

I have tested this with the following modifications to the Field Matcher struct:

type FieldMatcher struct {
	Field     string
	Modifiers []string
	Values    []interface{}
}

func (f *FieldMatcher) unmarshal(field *yaml.Node, values *yaml.Node) error {
	fieldParts := strings.Split(field.Value, "|")
	f.Field, f.Modifiers = fieldParts[0], fieldParts[1:]

	switch values.Kind {
	case yaml.ScalarNode:
		f.Values = []interface{}{nil}
		return values.Decode(&f.Values[0])
	case yaml.SequenceNode:
		return values.Decode(&f.Values)
	case yaml.MappingNode:
		fallthrough
	case yaml.DocumentNode:
		f.Values = []interface{}{map[string]interface{}{}}
		return values.Decode(&f.Values[0])
	case yaml.AliasNode:
		return f.unmarshal(field, values.Alias)
	}
	return nil
}

This works as long as you don't import evaluator, but breaks the evaluator implementation currently. Since I don't use the evaluator that is bundled here, I wanted to check if this was something you'd be interested in supporting before going any further down this road. To be clear, I'd be okay with making the necessary modifications to the evaluator to support/understand the interface{} value types myself, and submit a full PR. I just don't want to go down that path if it's not something you agree with as the project owner.

My use-case is basically that there are some things like |gt or |lt modifiers which operate on numeric values or more complex custom Sigma additions which require structured/typed data in the field matcher values. For example, internally we have a |nested modifier which interprets the value as a separate condition operating on nested documents within the event. This is impossible to implement since the current parser drops any values that aren't scalar.

If you're not interested here, I may end up maintaining an internal fork, but I'd love to push whatever I can back upstream if you're interested. 😃

I'm testing the latest sigma-go and it seems to be expecting the "related" key word to be followed by a string. But some rules have a map here.

% ./sigmatest proc_creation_win_apt_chafer_mar18.yml
2022/09/19 13:53:06 yaml: unmarshal errors:
  line 4: cannot unmarshal !!map into string

% head -5 proc_creation_win_apt_chafer_mar18.yml
title: Chafer Activity
id: ce6e34ca-966d-41c9-8d93-5b06c8b97a06
related:
    - id: 53ba33fd-3a50-4468-a5ef-c583635cfa92
      type: derived

https://github.com/SigmaHQ/sigma/blob/master/rules/windows/process_creation/proc_creation_win_apt_chafer_mar18.yml

proc_creation_win_apt_chafer_mar18.txt

As demonstrated in #25, the rule parser fails to find a match if a rule evaluator has multiple field mappings for one field, e.g if RuleEvaluator.fieldmappings looks like

"FieldToEval": ["$.payload.something.field", "$.payload.another.field"]

then the rule

detection:
    thingToMatch:
        FieldToEval: value
    condition: thingToMatch

will fail to match on an event like

{
    "payload": {
        "something": {
            "field": "value"
        }
    }
}

It looks like the library is supposed to be able to support multiple mappings for the same field, but if not then feel free to ignore.

I am running into an issue where matching is not done case insensitive. Here's the rule: https://github.com/SigmaHQ/sigma/blob/master/rules/windows/process_creation/proc_creation_win_susp_non_exe_image.yml

Here's the specific matching clause that is not working properly:

detection:
    known_image_extension:
        Image|endswith:
            - '.exe'
            - '.tmp' # sadly many installers use this extension
           
...
    condition: not known_image_extension and not 1 of filter*

Here's the input:

Image: "C:\\Windows\\System32\\SppExtComObj.Exe"

I would expect that to MATCH known_image_extension and therefore not match the condition.

However, it does NOT match known_image_extension as you can see here:

map[string]interface {}{"Image":"C:\\Windows\\System32\\SppExtComObj.Exe"}
event2 Match result {Match:true SearchResults:map[filter_com:false filter_emc_networker:false filter_empty:false filter_image:false filter_libreoffice:false filter_lzma_exe:false filter_msi_rollbackfiles:false filter_myq_server:false filter_null:false filter_nvidia:false filter_pstarts:false filter_screensaver:false filter_starts:false filter_visualstudio:false filter_vscode:false filter_winpakpro:false filter_winscp:false filter_wsl:false known_image_extension:false] ConditionResults:[true]}

However, if I change the rule to match the case it works properly:

detection:
    known_image_extension:
        Image|endswith:
            - '.Exe'
            - '.exe'
            - '.tmp' # sadly many installers use this extension

Then the same input matches known_image_extension and therefore does not match the condition:

map[string]interface {}{"Image":"C:\\Windows\\System32\\SppExtComObj.Exe"}
event2 Match result {Match:false SearchResults:map[filter_com:false filter_emc_networker:false filter_empty:false filter_image:false filter_libreoffice:false filter_lzma_exe:false filter_msi_rollbackfiles:false filter_myq_server:false filter_null:false filter_nvidia:false filter_pstarts:false filter_screensaver:false filter_starts:false filter_visualstudio:false filter_vscode:false filter_winpakpro:false filter_winscp:false filter_wsl:false known_image_extension:true] ConditionResults:[false]}

Hello! First off, this project looks great. Thanks for the work so far.

I'm curious if there is any appetite for:

1. External Contributions
2. Improvement on the comparator/modifier implementation.

I was evaluating this library for potential use but noticed that the comparison logic doesn't really comply with the Sigma specification. Specifically, it does not support the globbing patterns in standard field comparisons. I was also looking to see if we could customize this comparison logic because we have some internal requirements that may change the way rules are evaluated.

I'm currently working on some changes that would add a Comparator interface type which allows users of the library to implement their own comparison logic for a RuleEvaluator (and incidentally allow users to extend the library to support other modifiers or disallow certain modifiers). Right now, this looks something like this:

// Compare an expected value to a value found within an event
type CompareFunc func(value interface{}, expected interface{}) bool

type Comparator interface {
	// The base comparator with no modifiers
	Base(value interface{}, expected interface{}) bool
	// Retrieve the comparator for a given set of (possibly empty) modifiers.
	Get(modifiers []string) (CompareFunc, error)
}

With a default implementation which supports the same modifiers as the current implementation, but also adds support for globbing as defined by the spec. It is also a little more concrete, as it uses type-switching to verify the value and expected value types rather than farming out to fmt.Sprintf("%v") (which, imho, could produce unexpected/unwanted results for users who don't understand why/how their types are converting/matching/not matching).

In summary: Is there any appetite for potentially merging an interface like this? Or do you have any notes on the idea which may make the potential future PR more mergable from your perspective? I'm not super far into implementation or anything (basic structure is done, but I haven't done any serious testing yet). I just didn't see any contribution guidelines, and wanted to get a gut check before going to deep into the effort. Thanks! :)

Match() now returns a Results object and an err, not just a bool

Currently sigma-go panics when given a rule that looks like this:

detection:
    someCondition:
        foo: bar
    condition: nonExistentCondition

This panic happens here:

Instead this function should be extended so that it can return an error back to the caller

Could not parse this one:

detection:
  selection1:
    CommandLine|contains: 'setup0.exe -p'
  selection2a:
    CommandLine|contains: 'setup.exe'
  selection2b:
    CommandLine|endswith:
      - '-x:0'
      - '-x:1'
      - '-x:2'
  condition: selection1 or all of selection2*

invalid token '*'

https://github.com/SigmaHQ/sigma/blob/master/rules/windows/process_creation/proc_creation_win_apt_winnti_pipemon.yml

Add support for the cidr modifier described here: SigmaHQ/sigma#969

Hello! Amazing project and I appreciate your work.

Unfortunately, I encountered a couple of problems while attempting to use the project for a detection engine.

Given the following Sigma Rule:

title: Test Sigma Rule
id: 123
status: experimental
description: Crash the evaluator
date: 2024/03/27
detection:
  cond1:
    - receivedByte|gte: 0
  cond2:
    - deviceAddress|gte: 172.16.0.0
  condition: cond1 and cond2
level: low

and event:

{
  "receivedBytes": 25,
  "deviceAddress": "172.16.2.2"
}

the RuleEvaluator crashes the program in two ways:

1. if it does not find the receivedByte in the map[string]interface{} event, and passes to coerceNumeric a nil interface, which generates a panic.
2. if the rule contains a modifier that forces the use of the coerceNumeric and the value of the key is a string that can not be transformed to a float64 or int by yaml.Unmarshal(like an IP address), the coerceNumeric function recurses itself until a stack overflow.

I understand that writing a corect rule is paramount, but sometimes it can not be helped that the user makes a typo.

I can submit a pull request with the required fixes, if it helps.

While the Result type tells you which conditions matched in your rule, it doesn't help you know why they matched.

So e.g. for phish.report/IOK rules, you can have a long list of requests made by the page and so a condition:

dodgyRequest:
    request|endswith: 5844ad4.js

Doesn't help you identify the specific bad request.

Proposal to make a breaking API change to extend the result to include the matching value e.g.

type Result struct {
	Match            bool            // whether this event matches the Sigma rule
	SearchResults    map[string]bool // For each Search, whether it matched the event
	ConditionResults []bool          // For each Condition, whether it matched the event
}

type SearchResult struct {
	Matched bool
	MatchedValues map[string]interface{} // A map from field name to event value
}

Callers of the library can then present useful information to their users

Maybe just a documentation issue? or was the tool removed elsewhere?