From 326e0a44d1a2158f8e9bc32cc2f1e4ecb50354a8 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 9 May 2024 15:02:37 -0400 Subject: [PATCH] Reject CBOR simple values other than true, false, and null. --- .../cbor/internal/modes/appendixa_test.go | 3 --- .../serializer/cbor/internal/modes/decode.go | 25 +++++++++++++++++++ .../cbor/internal/modes/decode_test.go | 20 ++++++--------- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go index 7442e584079..50f6d648fab 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go @@ -249,17 +249,14 @@ func TestAppendixA(t *testing.T) { { example: hex("f7"), reject: "only simple values false, true, and null have a clear analog", - fixme: "the undefined simple value should not successfully decode as nil", }, { example: hex("f0"), reject: "only simple values false, true, and null have a clear analog", - fixme: "simple values other than false, true, and null should be rejected", }, { example: hex("f8ff"), reject: "only simple values false, true, and null have a clear analog", - fixme: "simple values other than false, true, and null should be rejected", }, { example: hex("c074323031332d30332d32315432303a30343a30305a"), diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go index 14c56970569..4dc19dd5750 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go @@ -22,6 +22,28 @@ import ( "github.com/fxamacker/cbor/v2" ) +var simpleValues *cbor.SimpleValueRegistry = func() *cbor.SimpleValueRegistry { + var opts []func(*cbor.SimpleValueRegistry) error + for sv := 0; sv <= 255; sv++ { + // Reject simple values 0-19, 23, and 32-255. The simple values 24-31 are reserved + // and considered ill-formed by the CBOR specification. We only accept false (20), + // true (21), and null (22). + switch sv { + case 20: // false + case 21: // true + case 22: // null + case 24, 25, 26, 27, 28, 29, 30, 31: // reserved + default: + opts = append(opts, cbor.WithRejectedSimpleValue(cbor.SimpleValue(sv))) + } + } + simpleValues, err := cbor.NewSimpleValueRegistryFromDefaults(opts...) + if err != nil { + panic(err) + } + return simpleValues +}() + var Decode cbor.DecMode = func() cbor.DecMode { decode, err := cbor.DecOptions{ // Maps with duplicate keys are well-formed but invalid according to the CBOR spec @@ -100,6 +122,9 @@ var Decode cbor.DecMode = func() cbor.DecMode { // Reject the arbitrary-precision integer tags because they can't be faithfully // roundtripped through the allowable Unstructured types. BignumTag: cbor.BignumTagForbidden, + + // Reject anything other than the simple values true, false, and null. + SimpleValues: simpleValues, }.DecMode() if err != nil { panic(err) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go index e10468a79d7..cd516803946 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go @@ -585,13 +585,11 @@ func TestDecode(t *testing.T) { { name: "simple value 23", in: hex("f7"), // undefined - assertOnError: func(t *testing.T, e error) { - // TODO: Once this can pass, make the assertion stronger. - if e == nil { - t.Error("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "simple value 23 is not recognized"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "cbor simple value 23 (\"undefined\") should not be accepted", + }), }, }, func() (generated []test) { // Generate test cases for all simple values (0 to 255) because the number of possible simple values is fixed and small. @@ -619,13 +617,11 @@ func TestDecode(t *testing.T) { }) default: // reject all unrecognized simple values - each.assertOnError = func(t *testing.T, e error) { - // TODO: Once this can pass, make the assertion stronger. - if e == nil { - t.Error("expected non-nil error") + each.assertOnError = assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: fmt.Sprintf("simple value %d is not recognized", i)}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - } - each.fixme = "unrecognized simple values should be rejected" + }) } generated = append(generated, each) }