Skip to content

Commit

Permalink
cmd/otk-*: do basic input validation to avoid crashing too easily
Browse files Browse the repository at this point in the history
This commit is a lesson from osbuild/otk#186
as it is right now extremly easy to crash the otk disk externals
with bad inputs.

This commit adds basic input validation, it's probably a good
idea to also go deeper into the library and make it less trusting
about pointers but that is (slightly) orthogonal.
  • Loading branch information
mvo5 authored and achilleas-k committed Sep 10, 2024
1 parent 4cc61ea commit dc9903b
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 0 deletions.
3 changes: 3 additions & 0 deletions cmd/otk-make-fstab-stage/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ func run(r io.Reader, w io.Writer) error {
if err := json.NewDecoder(r).Decode(&inp); err != nil {
return err
}
if err := inp.Tree.Validate(); err != nil {
return fmt.Errorf("cannot validate input data: %w", err)
}

opts, err := osbuild.NewFSTabStageOptions(inp.Tree.Const.Internal.PartitionTable)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions cmd/otk-make-fstab-stage/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,10 @@ func TestIntegration(t *testing.T) {

assert.Equal(t, expectedStages, fakeStdout.String())
}

func TestIntegrationNoPartitionTable(t *testing.T) {
fakeStdin := bytes.NewBufferString(`{}`)
fakeStdout := bytes.NewBuffer(nil)
err := makefstab.Run(fakeStdin, fakeStdout)
assert.EqualError(t, err, "cannot validate input data: no partition table")
}
3 changes: 3 additions & 0 deletions cmd/otk-make-partition-mounts-devices/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ func run(r io.Reader, w io.Writer) error {
if err := json.NewDecoder(r).Decode(&inp); err != nil {
return err
}
if err := inp.Tree.Validate(); err != nil {
return fmt.Errorf("cannot validate input data: %w", err)
}

rootMntName, mounts, devices, err := osbuild.GenMountsDevicesFromPT(inp.Tree.Const.Filename, inp.Tree.Const.Internal.PartitionTable)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions cmd/otk-make-partition-mounts-devices/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,10 @@ func TestIntegration(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, expectedOutput, fakeStdout.String())
}

func TestIntegrationNoPartitionTable(t *testing.T) {
fakeStdin := bytes.NewBufferString(`{}`)
fakeStdout := bytes.NewBuffer(nil)
err := mkdevmnt.Run(fakeStdin, fakeStdout)
assert.EqualError(t, err, "cannot validate input data: no partition table")
}
3 changes: 3 additions & 0 deletions cmd/otk-make-partition-stages/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func run(r io.Reader, w io.Writer) error {
if err := json.NewDecoder(r).Decode(&inp); err != nil {
return err
}
if err := inp.Tree.Validate(); err != nil {
return fmt.Errorf("cannot validate input data: %w", err)
}

stages, err := makeImagePrepareStages(inp, inp.Tree.Const.Filename)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions cmd/otk-make-partition-stages/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,10 @@ func TestModificationFname(t *testing.T) {

assert.Equal(t, expectedStages, fakeStdout.String())
}

func TestIntegrationNoPartitionTable(t *testing.T) {
fakeStdin := bytes.NewBufferString(`{}`)
fakeStdout := bytes.NewBuffer(nil)
err := makestages.Run(fakeStdin, fakeStdout)
assert.EqualError(t, err, "cannot validate input data: no partition table")
}
13 changes: 13 additions & 0 deletions internal/otkdisk/partition.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ type Data struct {
Const Const `json:"const"`
}

// Validate does basic validation of the data
func (d Data) Validate() error {
return d.Const.Internal.Validate()
}

// Const contains partition table data that is considered "constant",
// i.e. that should not be modified by the consumer as there may be
// inter-dependencies between the values
Expand Down Expand Up @@ -52,6 +57,14 @@ type Internal struct {
PartitionTable *disk.PartitionTable `json:"partition-table"`
}

// Validate does basic validation of the internal data
func (i Internal) Validate() error {
if i.PartitionTable == nil {
return fmt.Errorf("no partition table")
}
return nil
}

// PartType represents a partition type
type PartType string

Expand Down
11 changes: 11 additions & 0 deletions internal/otkdisk/partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/stretchr/testify/assert"

"github.com/osbuild/images/internal/otkdisk"
"github.com/osbuild/images/pkg/disk"
)

func TestPartTypeValidationHappy(t *testing.T) {
Expand All @@ -17,3 +18,13 @@ func TestPartTypeValidationSad(t *testing.T) {
assert.EqualError(t, otkdisk.PartTypeUnset.Validate(), `unsupported partition type ""`)
assert.EqualError(t, otkdisk.PartType("foo").Validate(), `unsupported partition type "foo"`)
}

func TestDataValidates(t *testing.T) {
// sad
d := otkdisk.Data{}
assert.EqualError(t, d.Validate(), "no partition table")
// happy
d.Const.Internal.PartitionTable = &disk.PartitionTable{}
err := d.Validate()
assert.NoError(t, err)
}

0 comments on commit dc9903b

Please sign in to comment.