Skip to content

Commit 4bf22b5

Browse files
committed
fix(builtin): guard against missing args in builtins
The type checker skips argument validation for calls where the callee has unknown type (e.g. method calls on any-typed values). This allowed builtins like join() to reach runtime with zero arguments, panicking on args[0] access. Add runtime len(args) guards to a number of builtin functions that previously accessed args elements without bounds checking. Also fix date() to check len(args) a second time after stripping a leading *time.Location argument. Relates to OSS-Fuzz finding in #941. Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
1 parent 851b241 commit 4bf22b5

File tree

3 files changed

+55
-7
lines changed

3 files changed

+55
-7
lines changed

builtin/builtin.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ var Builtins = []*Function{
222222
{
223223
Name: "trimPrefix",
224224
Func: func(args ...any) (any, error) {
225+
if len(args) == 0 {
226+
return nil, fmt.Errorf("not enough arguments to call trimPrefix")
227+
}
225228
s := " "
226229
if len(args) == 2 {
227230
s = args[1].(string)
@@ -236,6 +239,9 @@ var Builtins = []*Function{
236239
{
237240
Name: "trimSuffix",
238241
Func: func(args ...any) (any, error) {
242+
if len(args) == 0 {
243+
return nil, fmt.Errorf("not enough arguments to call trimSuffix")
244+
}
239245
s := " "
240246
if len(args) == 2 {
241247
s = args[1].(string)
@@ -312,6 +318,9 @@ var Builtins = []*Function{
312318
{
313319
Name: "repeat",
314320
Safe: func(args ...any) (any, uint, error) {
321+
if len(args) < 2 {
322+
return nil, 0, fmt.Errorf("not enough arguments to call repeat")
323+
}
315324
s := args[0].(string)
316325
n := runtime.ToInt(args[1])
317326
if n < 0 {
@@ -327,6 +336,9 @@ var Builtins = []*Function{
327336
{
328337
Name: "join",
329338
Func: func(args ...any) (any, error) {
339+
if len(args) == 0 {
340+
return nil, fmt.Errorf("not enough arguments to call join")
341+
}
330342
glue := ""
331343
if len(args) == 2 {
332344
glue = args[1].(string)
@@ -354,27 +366,39 @@ var Builtins = []*Function{
354366
{
355367
Name: "indexOf",
356368
Func: func(args ...any) (any, error) {
369+
if len(args) < 2 {
370+
return nil, fmt.Errorf("not enough arguments to call indexOf")
371+
}
357372
return strings.Index(args[0].(string), args[1].(string)), nil
358373
},
359374
Types: types(strings.Index),
360375
},
361376
{
362377
Name: "lastIndexOf",
363378
Func: func(args ...any) (any, error) {
379+
if len(args) < 2 {
380+
return nil, fmt.Errorf("not enough arguments to call lastIndexOf")
381+
}
364382
return strings.LastIndex(args[0].(string), args[1].(string)), nil
365383
},
366384
Types: types(strings.LastIndex),
367385
},
368386
{
369387
Name: "hasPrefix",
370388
Func: func(args ...any) (any, error) {
389+
if len(args) < 2 {
390+
return nil, fmt.Errorf("not enough arguments to call hasPrefix")
391+
}
371392
return strings.HasPrefix(args[0].(string), args[1].(string)), nil
372393
},
373394
Types: types(strings.HasPrefix),
374395
},
375396
{
376397
Name: "hasSuffix",
377398
Func: func(args ...any) (any, error) {
399+
if len(args) < 2 {
400+
return nil, fmt.Errorf("not enough arguments to call hasSuffix")
401+
}
378402
return strings.HasSuffix(args[0].(string), args[1].(string)), nil
379403
},
380404
Types: types(strings.HasSuffix),
@@ -436,6 +460,9 @@ var Builtins = []*Function{
436460
{
437461
Name: "toJSON",
438462
Func: func(args ...any) (any, error) {
463+
if len(args) == 0 {
464+
return nil, fmt.Errorf("not enough arguments to call toJSON")
465+
}
439466
b, err := json.MarshalIndent(args[0], "", " ")
440467
if err != nil {
441468
return nil, err
@@ -447,6 +474,9 @@ var Builtins = []*Function{
447474
{
448475
Name: "fromJSON",
449476
Func: func(args ...any) (any, error) {
477+
if len(args) == 0 {
478+
return nil, fmt.Errorf("not enough arguments to call fromJSON")
479+
}
450480
var v any
451481
err := json.Unmarshal([]byte(args[0].(string)), &v)
452482
if err != nil {
@@ -459,13 +489,19 @@ var Builtins = []*Function{
459489
{
460490
Name: "toBase64",
461491
Func: func(args ...any) (any, error) {
492+
if len(args) == 0 {
493+
return nil, fmt.Errorf("not enough arguments to call toBase64")
494+
}
462495
return base64.StdEncoding.EncodeToString([]byte(args[0].(string))), nil
463496
},
464497
Types: types(new(func(string) string)),
465498
},
466499
{
467500
Name: "fromBase64",
468501
Func: func(args ...any) (any, error) {
502+
if len(args) == 0 {
503+
return nil, fmt.Errorf("not enough arguments to call fromBase64")
504+
}
469505
b, err := base64.StdEncoding.DecodeString(args[0].(string))
470506
if err != nil {
471507
return nil, err
@@ -505,17 +541,26 @@ var Builtins = []*Function{
505541
{
506542
Name: "duration",
507543
Func: func(args ...any) (any, error) {
544+
if len(args) == 0 {
545+
return nil, fmt.Errorf("not enough arguments to call duration")
546+
}
508547
return time.ParseDuration(args[0].(string))
509548
},
510549
Types: types(time.ParseDuration),
511550
},
512551
{
513552
Name: "date",
514553
Func: func(args ...any) (any, error) {
554+
if len(args) == 0 {
555+
return nil, fmt.Errorf("not enough arguments to call date")
556+
}
515557
tz, ok := args[0].(*time.Location)
516558
if ok {
517559
args = args[1:]
518560
}
561+
if len(args) == 0 {
562+
return nil, fmt.Errorf("not enough arguments to call date")
563+
}
519564

520565
date := args[0].(string)
521566
if len(args) == 2 {
@@ -585,6 +630,9 @@ var Builtins = []*Function{
585630
{
586631
Name: "timezone",
587632
Func: func(args ...any) (any, error) {
633+
if len(args) == 0 {
634+
return nil, fmt.Errorf("not enough arguments to call timezone")
635+
}
588636
tz, err := time.LoadLocation(args[0].(string))
589637
if err != nil {
590638
return nil, err
@@ -596,6 +644,9 @@ var Builtins = []*Function{
596644
{
597645
Name: "first",
598646
Func: func(args ...any) (any, error) {
647+
if len(args) != 1 {
648+
return nil, fmt.Errorf("invalid number of arguments (expected 1, got %d)", len(args))
649+
}
599650
defer func() {
600651
if r := recover(); r != nil {
601652
return
@@ -619,6 +670,9 @@ var Builtins = []*Function{
619670
{
620671
Name: "last",
621672
Func: func(args ...any) (any, error) {
673+
if len(args) != 1 {
674+
return nil, fmt.Errorf("invalid number of arguments (expected 1, got %d)", len(args))
675+
}
622676
defer func() {
623677
if r := recover(); r != nil {
624678
return

test/fuzz/fuzz_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ func FuzzExpr(f *testing.F) {
6666
regexp.MustCompile(`invalid order .*, expected asc or desc`),
6767
regexp.MustCompile(`unknown order, use asc or desc`),
6868
regexp.MustCompile(`cannot use .* as a key for groupBy: type is not comparable`),
69+
regexp.MustCompile(`not enough arguments to call .*`),
6970
}
7071

7172
env := NewEnv()

testdata/generated.txt

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,6 @@ $env not contains $env?.[Bar]?.add()
763763
$env not contains $env?.[Bar]?.ok
764764
$env not contains $env?.[String?.str]
765765
$env not contains $env?.[String]
766-
$env not contains $env?.[first(foobar, greet)]
767766
$env not contains $env?.[foobar?.array()]
768767
$env not contains $env?.[foobar]
769768
$env not contains $env?.[toJSON(foobar)]
@@ -3513,7 +3512,6 @@ $env?.[Bar]?.str(f64)
35133512
$env?.[Bar]?.str(f64, array)
35143513
$env?.[Bar]?.str(foobar)
35153514
$env?.[Bar]?.str(greet)
3516-
$env?.[Bar]?.str(list | first(true, foobar))
35173515
$env?.[Bar]?.str(list)
35183516
$env?.[Bar]?.str(nil)
35193517
$env?.[Bar]?.str.Bar
@@ -3645,7 +3643,6 @@ $env?.[String]?.String(foobar)
36453643
$env?.[String]?.String(foobar, $env)
36463644
$env?.[String]?.String(foobar?.[list])?.str()
36473645
$env?.[String]?.String(greet)
3648-
$env?.[String]?.String(last($env, String))
36493646
$env?.[String]?.String(ok && foobar)
36503647
$env?.[String]?.String.add
36513648
$env?.[String]?.String.split(false, add).str.array
@@ -6984,7 +6981,6 @@ $env[:str] not in foo || true
69846981
0 == $env?.[Bar | get(foo)]
69856982
0 == $env?.[Bar]
69866983
0 == $env?.[String]
6987-
0 == $env?.[first(0, ok)]
69886984
0 == $env?.[foobar]
69896985
0 == $env?.[foobar]?.add
69906986
0 == $env?.[str]
@@ -18596,7 +18592,6 @@ foo not in $env?.[foobar?.f64]
1859618592
foo not in $env?.[foobar?.foo]
1859718593
foo not in $env?.[foobar]
1859818594
foo not in $env?.[foobar]?.[greet]
18599-
foo not in $env?.[nil | last(foobar)]
1860018595
foo not in $env?.[nil]
1860118596
foo not in $env?.foobar
1860218597
foo not in $env?.foobar?.greet(foobar)
@@ -28092,7 +28087,6 @@ nil != $env?.Bar?.list()
2809228087
nil != $env?.Bar?.str()
2809328088
nil != $env?.String
2809428089
nil != $env?.String?.[i]
28095-
nil != $env?.[1.0 | first(nil)]
2809628090
nil != $env?.[Bar]
2809728091
nil != $env?.[Bar]?.Bar
2809828092
nil != $env?.[String]
@@ -36587,7 +36581,6 @@ true in $env?.String
3658736581
true in $env?.[Bar]
3658836582
true in $env?.[String]
3658936583
true in $env?.[first(foobar)]
36590-
true in $env?.[foobar | last(nil)]
3659136584
true in $env?.[foobar]
3659236585
true not in $env?.Bar
3659336586
true not in $env?.String

0 commit comments

Comments
 (0)