diff --git a/internal/boxfile/boxfile.go b/internal/boxfile/boxfile.go index dceae9a..b467ae5 100644 --- a/internal/boxfile/boxfile.go +++ b/internal/boxfile/boxfile.go @@ -160,8 +160,7 @@ func Parse(path string) (Spec, error) { // Decoding empty input clears the destination, wiping the defaults, so // skip it entirely: a Boxfile with no content means "all defaults". if len(bytes.TrimSpace(b)) > 0 { - dec := yaml.NewDecoder(bytes.NewReader(b), yaml.DisallowUnknownField()) - if err := dec.Decode(&s); err != nil && !errors.Is(err, io.EOF) { + if err := decode(b, &s); err != nil { return Spec{}, fmt.Errorf("%s: %w", path, err) } } @@ -171,6 +170,22 @@ func Parse(path string) (Spec, error) { return s, nil } +// decode reads YAML into s. The YAML library panics on some malformed input +// (a bare "!" tag on a list, for one), so a panic is reported as a parse +// error rather than taking down box, or a server, with it. +func decode(b []byte, s *Spec) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("cannot parse: %v", r) + } + }() + dec := yaml.NewDecoder(bytes.NewReader(b), yaml.DisallowUnknownField()) + if err := dec.Decode(s); err != nil && !errors.Is(err, io.EOF) { + return err + } + return nil +} + // Grammar rules for spec fields that are data, not instructions. A value // violating its rule could otherwise change the structure of the generated // Containerfile (a newline in an env value becomes a new instruction, a mode diff --git a/internal/boxfile/boxfile_test.go b/internal/boxfile/boxfile_test.go index cd76f12..32166b0 100644 --- a/internal/boxfile/boxfile_test.go +++ b/internal/boxfile/boxfile_test.go @@ -362,3 +362,13 @@ func TestIsolation(t *testing.T) { } } } + +// The YAML library panics on a bare tag where a list is expected; Parse must +// turn that into an error. Found by FuzzParse. +func TestParseSurvivesYAMLPanic(t *testing.T) { + for _, body := range []string{"base: alpine\npackages: !\n", "base: alpine\npackages:\n ! 0\n"} { + if _, err := Parse(write(t, body)); err == nil { + t.Errorf("Parse(%q) should fail", body) + } + } +} diff --git a/internal/boxfile/testdata/fuzz/FuzzParse/57d31b51e80d8201 b/internal/boxfile/testdata/fuzz/FuzzParse/57d31b51e80d8201 new file mode 100644 index 0000000..874176d --- /dev/null +++ b/internal/boxfile/testdata/fuzz/FuzzParse/57d31b51e80d8201 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte(" \n\nbase: .\n\n\nram_mib: 1\nnetwork: A\n\nA: false #\nisolation: A #\nbackend: A #\ntimeout_seconds: A #\nwarm: 0 #\n\n#\npackages:\n ! 0000000000000")