fix(checker): validate sortBy order argument type (#912)

The sortBy and sort functions accept an optional order argument that
must be "asc" or "desc". Previously, passing a non-string value would
cause a panic due to an unsafe type assertion in the VM. This adds
compile-time validation in the type checker and defensive runtime
checks in both the VM and builtin implementations to return proper
errors instead of panicking.

Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>

Author: thevilledev

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)
 				}
 			}
 

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() &&

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 <nil> 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()

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

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":

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{