From 28272e6900d37a204121332b1be28416bcef4d8c Mon Sep 17 00:00:00 2001 From: Fabio Bozzo Date: Mon, 2 Dec 2024 11:59:16 +0100 Subject: [PATCH] move int validation to where a error can be returned --- pkg/policy/ipld.go | 5 ++ pkg/policy/limits/constants.go | 8 --- pkg/policy/limits/int.go | 49 ++++++++++++++++++ pkg/policy/limits/int_test.go | 82 ++++++++++++++++++++++++++++++ pkg/policy/literal/literal.go | 12 ++++- pkg/policy/literal/literal_test.go | 10 ++-- pkg/policy/match.go | 6 --- pkg/policy/match_test.go | 36 ------------- pkg/policy/selector/selector.go | 5 ++ 9 files changed, 156 insertions(+), 57 deletions(-) delete mode 100644 pkg/policy/limits/constants.go create mode 100644 pkg/policy/limits/int.go create mode 100644 pkg/policy/limits/int_test.go diff --git a/pkg/policy/ipld.go b/pkg/policy/ipld.go index 9d52d4d..752dd96 100644 --- a/pkg/policy/ipld.go +++ b/pkg/policy/ipld.go @@ -9,10 +9,15 @@ import ( "github.com/ipld/go-ipld-prime/must" "github.com/ipld/go-ipld-prime/node/basicnode" + "github.com/ucan-wg/go-ucan/pkg/policy/limits" "github.com/ucan-wg/go-ucan/pkg/policy/selector" ) func FromIPLD(node datamodel.Node) (Policy, error) { + if err := limits.ValidateIntegerBoundsIPLD(node); err != nil { + return nil, fmt.Errorf("policy contains integer values outside safe bounds: %w", err) + } + return statementsFromIPLD("/", node) } diff --git a/pkg/policy/limits/constants.go b/pkg/policy/limits/constants.go deleted file mode 100644 index c2d1fd5..0000000 --- a/pkg/policy/limits/constants.go +++ /dev/null @@ -1,8 +0,0 @@ -package limits - -const ( - // MaxInt53 represents the maximum safe integer in JavaScript (2^53 - 1) - MaxInt53 = 9007199254740991 - // MinInt53 represents the minimum safe integer in JavaScript (-2^53 + 1) - MinInt53 = -9007199254740991 -) diff --git a/pkg/policy/limits/int.go b/pkg/policy/limits/int.go new file mode 100644 index 0000000..91dd135 --- /dev/null +++ b/pkg/policy/limits/int.go @@ -0,0 +1,49 @@ +package limits + +import ( + "fmt" + + "github.com/ipld/go-ipld-prime" + "github.com/ipld/go-ipld-prime/must" +) + +const ( + // MaxInt53 represents the maximum safe integer in JavaScript (2^53 - 1) + MaxInt53 = 9007199254740991 + // MinInt53 represents the minimum safe integer in JavaScript (-2^53 + 1) + MinInt53 = -9007199254740991 +) + +func ValidateIntegerBoundsIPLD(node ipld.Node) error { + switch node.Kind() { + case ipld.Kind_Int: + val := must.Int(node) + if val > MaxInt53 || val < MinInt53 { + return fmt.Errorf("integer value %d exceeds safe bounds", val) + } + case ipld.Kind_List: + it := node.ListIterator() + for !it.Done() { + _, v, err := it.Next() + if err != nil { + return err + } + if err := ValidateIntegerBoundsIPLD(v); err != nil { + return err + } + } + case ipld.Kind_Map: + it := node.MapIterator() + for !it.Done() { + _, v, err := it.Next() + if err != nil { + return err + } + if err := ValidateIntegerBoundsIPLD(v); err != nil { + return err + } + } + } + + return nil +} diff --git a/pkg/policy/limits/int_test.go b/pkg/policy/limits/int_test.go new file mode 100644 index 0000000..de3f21e --- /dev/null +++ b/pkg/policy/limits/int_test.go @@ -0,0 +1,82 @@ +package limits + +import ( + "testing" + + "github.com/ipld/go-ipld-prime/datamodel" + "github.com/ipld/go-ipld-prime/fluent/qp" + "github.com/ipld/go-ipld-prime/node/basicnode" + "github.com/stretchr/testify/require" +) + +func TestValidateIntegerBoundsIPLD(t *testing.T) { + buildMap := func() datamodel.Node { + nb := basicnode.Prototype.Any.NewBuilder() + qp.Map(1, func(ma datamodel.MapAssembler) { + qp.MapEntry(ma, "foo", qp.Int(MaxInt53+1)) + })(nb) + return nb.Build() + } + + buildList := func() datamodel.Node { + nb := basicnode.Prototype.Any.NewBuilder() + qp.List(1, func(la datamodel.ListAssembler) { + qp.ListEntry(la, qp.Int(MinInt53-1)) + })(nb) + return nb.Build() + } + + tests := []struct { + name string + input datamodel.Node + wantErr bool + }{ + { + name: "valid int", + input: basicnode.NewInt(42), + wantErr: false, + }, + { + name: "max safe int", + input: basicnode.NewInt(MaxInt53), + wantErr: false, + }, + { + name: "min safe int", + input: basicnode.NewInt(MinInt53), + wantErr: false, + }, + { + name: "above MaxInt53", + input: basicnode.NewInt(MaxInt53 + 1), + wantErr: true, + }, + { + name: "below MinInt53", + input: basicnode.NewInt(MinInt53 - 1), + wantErr: true, + }, + { + name: "nested map with invalid int", + input: buildMap(), + wantErr: true, + }, + { + name: "nested list with invalid int", + input: buildList(), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateIntegerBoundsIPLD(tt.input) + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), "exceeds safe bounds") + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/policy/literal/literal.go b/pkg/policy/literal/literal.go index 618f33d..fcb03cb 100644 --- a/pkg/policy/literal/literal.go +++ b/pkg/policy/literal/literal.go @@ -69,7 +69,11 @@ func Any(v any) (res ipld.Node, err error) { case string: return basicnode.NewString(val), nil case int: - return basicnode.NewInt(int64(val)), nil + i := int64(val) + if i > limits.MaxInt53 || i < limits.MinInt53 { + return nil, fmt.Errorf("integer value %d exceeds safe integer bounds", i) + } + return basicnode.NewInt(i), nil case int8: return basicnode.NewInt(int64(val)), nil case int16: @@ -77,6 +81,9 @@ func Any(v any) (res ipld.Node, err error) { case int32: return basicnode.NewInt(int64(val)), nil case int64: + if val > limits.MaxInt53 || val < limits.MinInt53 { + return nil, fmt.Errorf("integer value %d exceeds safe integer bounds", val) + } return basicnode.NewInt(val), nil case uint: return basicnode.NewInt(int64(val)), nil @@ -87,6 +94,9 @@ func Any(v any) (res ipld.Node, err error) { case uint32: return basicnode.NewInt(int64(val)), nil case uint64: + if val > uint64(limits.MaxInt53) { + return nil, fmt.Errorf("unsigned integer value %d exceeds safe integer bounds", val) + } return basicnode.NewInt(int64(val)), nil case float32: return basicnode.NewFloat(float64(val)), nil diff --git a/pkg/policy/literal/literal_test.go b/pkg/policy/literal/literal_test.go index d2282d0..9f7e1f2 100644 --- a/pkg/policy/literal/literal_test.go +++ b/pkg/policy/literal/literal_test.go @@ -8,6 +8,7 @@ import ( cidlink "github.com/ipld/go-ipld-prime/linking/cid" "github.com/ipld/go-ipld-prime/printer" "github.com/stretchr/testify/require" + "github.com/ucan-wg/go-ucan/pkg/policy/limits" ) @@ -295,14 +296,11 @@ func TestAnyAssembleIntegerOverflow(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + _, err := Any(tt.input) if tt.shouldErr { - require.Panics(t, func() { - anyAssemble(tt.input) - }) + require.Error(t, err) } else { - require.NotPanics(t, func() { - anyAssemble(tt.input) - }) + require.NoError(t, err) } }) } diff --git a/pkg/policy/match.go b/pkg/policy/match.go index 81a5431..648b877 100644 --- a/pkg/policy/match.go +++ b/pkg/policy/match.go @@ -8,8 +8,6 @@ import ( "github.com/ipld/go-ipld-prime" "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/must" - - "github.com/ucan-wg/go-ucan/pkg/policy/limits" ) // Match determines if the IPLD node satisfies the policy. @@ -268,10 +266,6 @@ func isOrdered(expected ipld.Node, actual ipld.Node, satisfies func(order int) b a := must.Int(actual) b := must.Int(expected) - if a > limits.MaxInt53 || a < limits.MinInt53 || b > limits.MaxInt53 || b < limits.MinInt53 { - return false - } - return satisfies(cmp.Compare(a, b)) } diff --git a/pkg/policy/match_test.go b/pkg/policy/match_test.go index 89bf02d..108037a 100644 --- a/pkg/policy/match_test.go +++ b/pkg/policy/match_test.go @@ -11,7 +11,6 @@ import ( "github.com/ipld/go-ipld-prime/node/basicnode" "github.com/stretchr/testify/require" - "github.com/ucan-wg/go-ucan/pkg/policy/limits" "github.com/ucan-wg/go-ucan/pkg/policy/literal" ) @@ -902,38 +901,3 @@ func TestPartialMatch(t *testing.T) { }) } } - -func TestIntegerOverflow(t *testing.T) { - tests := []struct { - name string - expected ipld.Node - actual ipld.Node - want bool - }{ - { - name: "valid integers", - expected: literal.Int(42), - actual: literal.Int(43), - want: true, // for gt comparison - }, - { - name: "exceeds MaxInt53", - expected: literal.Int(limits.MaxInt53 + 1), - actual: literal.Int(42), - want: false, - }, - { - name: "below MinInt53", - expected: literal.Int(limits.MinInt53 - 1), - actual: literal.Int(42), - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := isOrdered(tt.expected, tt.actual, gt) - require.Equal(t, tt.want, result) - }) - } -} diff --git a/pkg/policy/selector/selector.go b/pkg/policy/selector/selector.go index 249cd44..9c3c2ef 100644 --- a/pkg/policy/selector/selector.go +++ b/pkg/policy/selector/selector.go @@ -10,6 +10,7 @@ import ( "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/fluent/qp" "github.com/ipld/go-ipld-prime/node/basicnode" + "github.com/ucan-wg/go-ucan/pkg/policy/limits" ) // Selector describes a UCAN policy selector, as specified here: @@ -22,6 +23,10 @@ type Selector []segment // - a resolutionerr error if not being able to resolve to a node // - nil and no errors, if the selector couldn't match on an optional segment (with ?). func (s Selector) Select(subject ipld.Node) (ipld.Node, error) { + if err := limits.ValidateIntegerBoundsIPLD(subject); err != nil { + return nil, fmt.Errorf("node contains integer values outside safe bounds: %w", err) + } + return resolve(s, subject, nil) }