diff --git a/builtin/builtin.go b/builtin/builtin.go index 4fe356fb..78b55be6 100644 --- a/builtin/builtin.go +++ b/builtin/builtin.go @@ -1000,13 +1000,17 @@ var Builtins = []*Function{ var desc bool if len(args) == 2 { - switch args[1].(string) { + order, ok := args[1].(string) + if !ok { + return nil, 0, fmt.Errorf("sort order argument must be a string (got %T)", args[1]) + } + switch order { case "asc": desc = false case "desc": desc = true default: - return nil, 0, fmt.Errorf("invalid order %s, expected asc or desc", args[1]) + return nil, 0, fmt.Errorf("invalid order %s, expected asc or desc", order) } } diff --git a/checker/checker.go b/checker/checker.go index b78afc2b..23ed1725 100644 --- a/checker/checker.go +++ b/checker/checker.go @@ -15,15 +15,15 @@ import ( ) var ( - anyType = reflect.TypeOf(new(any)).Elem() - boolType = reflect.TypeOf(true) - intType = reflect.TypeOf(0) - floatType = reflect.TypeOf(float64(0)) - stringType = reflect.TypeOf("") - arrayType = reflect.TypeOf([]any{}) - mapType = reflect.TypeOf(map[string]any{}) - timeType = reflect.TypeOf(time.Time{}) - durationType = reflect.TypeOf(time.Duration(0)) + anyType = reflect.TypeOf(new(any)).Elem() + boolType = reflect.TypeOf(true) + intType = reflect.TypeOf(0) + floatType = reflect.TypeOf(float64(0)) + stringType = reflect.TypeOf("") + arrayType = reflect.TypeOf([]any{}) + mapType = reflect.TypeOf(map[string]any{}) + timeType = reflect.TypeOf(time.Time{}) + durationType = reflect.TypeOf(time.Duration(0)) byteSliceType = reflect.TypeOf([]byte(nil)) anyTypeSlice = []reflect.Type{anyType} @@ -893,7 +893,10 @@ func (v *Checker) builtinNode(node *ast.BuiltinNode) Nature { v.end() if len(node.Arguments) == 3 { - _ = v.visit(node.Arguments[2]) + order := v.visit(node.Arguments[2]) + if !order.IsString() && !order.IsUnknown(&v.config.NtCache) { + return v.error(node.Arguments[2], "sortBy order argument must be a string (got %v)", order.String()) + } } if predicate.IsFunc() && diff --git a/test/fuzz/fuzz_test.go b/test/fuzz/fuzz_test.go index e24b96cd..09b4d4f6 100644 --- a/test/fuzz/fuzz_test.go +++ b/test/fuzz/fuzz_test.go @@ -57,6 +57,10 @@ func FuzzExpr(f *testing.F) { regexp.MustCompile(`reduce of empty array with no initial value`), regexp.MustCompile(`cannot use as argument \(type .*\) to call .*`), regexp.MustCompile(`illegal base64 data at input byte .*`), + regexp.MustCompile(`sort order argument must be a string`), + regexp.MustCompile(`sortBy order argument must be a string`), + regexp.MustCompile(`invalid order .*, expected asc or desc`), + regexp.MustCompile(`unknown order, use asc or desc`), } env := NewEnv() diff --git a/testdata/generated.txt b/testdata/generated.txt index 017cf580..5166aeb8 100644 --- a/testdata/generated.txt +++ b/testdata/generated.txt @@ -1004,7 +1004,6 @@ $env | reduce(false, $env); list $env | reduce(greet, array) ?? foo $env | reduce(ok, 1) ?: greet $env | reduce(str, foo) not startsWith str -$env | sortBy(#, 0) * $env && false $env | sum(#) not endsWith $env or true $env | sum(1.0) <= i $env | sum(1.0) in array @@ -15360,7 +15359,6 @@ f64 ?? reduce(list, str) f64 ?? reverse($env) f64 ?? reverse(array) f64 ?? round(i) -f64 ?? sortBy($env, #.String, list) f64 ?? sortBy(list, ok) f64 ?? str f64 ?? str[$env:i] @@ -30051,7 +30049,6 @@ ok || ok || $env ok || one($env, .add) ok || reduce($env, .foo) ok || sortBy($env, array).str -ok || sortBy($env, foo, 1) ok || sortBy($env, i, str) ok || str != $env ok || str != nil diff --git a/vm/vm.go b/vm/vm.go index 69c379b2..88782541 100644 --- a/vm/vm.go +++ b/vm/vm.go @@ -556,7 +556,11 @@ func (vm *VM) Run(program *Program, env any) (_ any, err error) { case 2: scope := vm.scope() var desc bool - switch vm.pop().(string) { + order, ok := vm.pop().(string) + if !ok { + panic("sortBy order argument must be a string") + } + switch order { case "asc": desc = false case "desc": diff --git a/vm/vm_test.go b/vm/vm_test.go index 355f764e..6963803e 100644 --- a/vm/vm_test.go +++ b/vm/vm_test.go @@ -513,6 +513,26 @@ func TestVM_GroupAndSortOperations(t *testing.T) { } } +// TestVM_SortBy_NonStringOrder tests that sortBy with non-string order +// returns a proper error instead of panicking (regression test for OSS-Fuzz #477658245). +func TestVM_SortBy_NonStringOrder(t *testing.T) { + env := map[string]any{} + fn := expr.Function("fn", func(params ...any) (any, error) { + return fmt.Sprintf("fn(%v)", params), nil + }) + + // This expression passes a function result as the order argument to sortBy. + // The function returns a string that is not "asc" or "desc", which should + // produce a proper error rather than a panic. + program, err := expr.Compile(`sortBy([1, 2, 3], #, fn($env))`, expr.Env(env), fn) + require.NoError(t, err) + + testVM := &vm.VM{} + _, err = testVM.Run(program, env) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown order") +} + // TestVM_ProfileOperations tests the profiling opcodes func TestVM_ProfileOperations(t *testing.T) { program := &vm.Program{