Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions builtin/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
23 changes: 13 additions & 10 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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() &&
Expand Down
4 changes: 4 additions & 0 deletions test/fuzz/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 0 additions & 3 deletions testdata/generated.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion vm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
20 changes: 20 additions & 0 deletions vm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
Loading