Skip to content

Commit

Permalink
fixes panic/stackoverflow when unparseable values passed to coerceNum…
Browse files Browse the repository at this point in the history
…eric (#43)
  • Loading branch information
262nos authored Apr 17, 2024
1 parent 7269de6 commit 600f555
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 16 deletions.
15 changes: 14 additions & 1 deletion evaluator/modifiers/modifiers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package modifiers
import (
"encoding/base64"
"fmt"
"gopkg.in/yaml.v3"
"net"
"reflect"
"regexp"
"strings"

"gopkg.in/yaml.v3"
)

func GetComparator(modifiers ...string) (ComparatorFunc, error) {
Expand Down Expand Up @@ -239,6 +240,10 @@ func coerceString(v interface{}) string {

// coerceNumeric makes both operands into the widest possible number of the same type
func coerceNumeric(left, right interface{}) (interface{}, interface{}, error) {
// Check for nil interface, otherwise the function panics
if left == nil || right == nil {
return nil, nil, fmt.Errorf("cannot coerce %T and %T to numeric", left, right)
}
leftV := reflect.ValueOf(left)
leftType := reflect.ValueOf(left).Type()
rightV := reflect.ValueOf(right)
Expand All @@ -265,12 +270,20 @@ func coerceNumeric(left, right interface{}) (interface{}, interface{}, error) {
if err := yaml.Unmarshal([]byte(left.(string)), &leftParsed); err != nil {
return nil, nil, err
}
//Check the parsed type is the correct one, otherwise we get a stack overflow
if reflect.TypeOf(leftParsed).Kind() != reflect.Float64 && reflect.TypeOf(leftParsed).Kind() != reflect.Int {
return nil, nil, fmt.Errorf("cannot coerce %T and %T to numeric", left, right)
}
return coerceNumeric(leftParsed, right)
case rightType.Kind() == reflect.String:
var rightParsed interface{}
if err := yaml.Unmarshal([]byte(right.(string)), &rightParsed); err != nil {
return nil, nil, err
}
//Check the parsed type is the correct one, otherwise we get a stack overflow
if reflect.TypeOf(rightParsed).Kind() != reflect.Float64 && reflect.TypeOf(rightParsed).Kind() != reflect.Int {
return nil, nil, fmt.Errorf("cannot coerce %T and %T to numeric", left, right)
}
return coerceNumeric(left, rightParsed)

default:
Expand Down
43 changes: 28 additions & 15 deletions evaluator/modifiers/modifiers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,40 @@ import (

func Test_compareNumeric(t *testing.T) {
tests := []struct {
left interface{}
right interface{}
wantGt bool
wantGte bool
wantLt bool
wantLte bool
left interface{}
right interface{}
wantGt bool
wantGte bool
wantLt bool
wantLte bool
shouldFail bool
}{
{1, 2, false, false, true, true},
{1.1, 1.2, false, false, true, true},
{1, 1.2, false, false, true, true},
{1.1, 2, false, false, true, true},
{1, "2", false, false, true, true},
{"1.1", 1.2, false, false, true, true},
{"1.1", 1.1, false, true, false, true},
{1, 2, false, false, true, true, false},
{1.1, 1.2, false, false, true, true, false},
{1, 1.2, false, false, true, true, false},
{1.1, 2, false, false, true, true, false},
{1, "2", false, false, true, true, false},
{"1.1", 1.2, false, false, true, true, false},
{"1.1", 1.1, false, true, false, true, false},

// The function panics if it's interfaces are nil, this happens if it doesn't find the field in the event and it's compared to a int or float
{nil, 2, true, false, false, false, true},
{nil, nil, true, false, false, false, true},
{2, nil, true, false, false, false, true},
// If we pass anything (like an ip address) other than an int or float, the functions recurses until it stack overflows
{"127.0.0.1", "127.0.0.1", true, false, false, false, true},
{"127.0.0.1", 0.2, true, false, false, false, true},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("%s_%s", tt.left, tt.right), func(t *testing.T) {
gotGt, gotGte, gotLt, gotLte, err := compareNumeric(tt.left, tt.right)
if err != nil {
t.Errorf("compareNumeric() error = %v", err)
return
if !tt.shouldFail {
t.Errorf("compareNumeric() error = %v", err)
return
} else {
return
}
}
if gotGt != tt.wantGt {
t.Errorf("compareNumeric() gotGt = %v, want %v", gotGt, tt.wantGt)
Expand Down

0 comments on commit 600f555

Please sign in to comment.