Merge pull request #1488 from nyaruka/number_fix

Add range check for numbers to prevent DynamoDB overflow

Author: rowanseymour

excellent/base_test.go

@@ -439,6 +439,10 @@ func TestEvaluationErrors(t *testing.T) {
 		{`@(format_datetime("x"))`, `error evaluating @(format_datetime("x")): error calling format_datetime(...): unable to convert "x" to a datetime`},
 		{`@(format_datetime(3))`, `error evaluating @(format_datetime(3)): error calling format_datetime(...): unable to convert 3 to a datetime`},
 
+		// number range errors
+		{`@(2 ^ 400)`, `error evaluating @(2 ^ 400): number value out of range`},
+		{`@(1234567890123456789012345678901234567)`, `error evaluating @(1234567890123456789012345678901234567): number 1234567890123456789012345678901234567 is out of range`},
+
 		// function call errors
 		{`@(FOO())`, `error evaluating @(FOO()): foo is not a function`},
 		{`@(count(1))`, `error evaluating @(count(1)): error calling count(...): value isn't countable`},

excellent/functions/builtin_test.go

@@ -622,6 +622,7 @@ func TestFunctions(t *testing.T) {
 
 		{"sum", dmy, []types.XValue{xa(xn("1"), xn("2"), xs("3"))}, xn("6")},
 		{"sum", dmy, []types.XValue{xa()}, xn("0")},
+		{"sum", dmy, []types.XValue{xa(xn("999999999999999999999999999999999999"), xn("999999999999999999999999999999999999"))}, ERROR}, // overflow
 		{"sum", dmy, []types.XValue{xs("xx")}, ERROR},
 		{"sum", dmy, []types.XValue{xa(xn("1"), xn("2"), xs("xx"))}, ERROR},
 		{"sum", dmy, []types.XValue{xa(xn("1"), xn("2"), ERROR)}, ERROR},

excellent/operators/builtin.go

@@ -100,6 +100,7 @@ var Divide = numericalBinary(func(env envs.Environment, num1 *types.XNumber, num
 // Exponent raises a number to the power of a another number.
 //
 //	@(2 ^ 8) -> 256
+//	@(2 ^ 400) -> ERROR
 //
 // @operator exponent "^"
 var Exponent = numericalBinary(func(env envs.Environment, num1 *types.XNumber, num2 *types.XNumber) types.XValue {

excellent/operators/builtin_test.go

@@ -73,9 +73,14 @@ func TestBinaryOperators(t *testing.T) {
 		{operators.Exponent, xn("2"), xn("32.000"), xn("4294967296")},
 		{operators.Exponent, xn("9"), xn("0.5"), xn("3")},
 		{operators.Exponent, xn("4"), xn("2.5"), xn("32")},
+		{operators.Exponent, xn("2"), xn("400"), ERROR}, // overflow
 		{operators.Exponent, ERROR, xi(1), ERROR},
 		{operators.Exponent, xi(1), ERROR, ERROR},
 
+		// overflow cases
+		{operators.Multiply, xn("9999999999999999999"), xn("9999999999999999999"), ERROR}, // product > 36 digits
+		{operators.Add, xn("999999999999999999999999999999999999"), xn("999999999999999999999999999999999999"), ERROR}, // sum > 36 significant digits
+
 		{operators.LessThan, xi(2), xi(3), types.XBooleanTrue},
 		{operators.LessThan, xi(3), xi(3), types.XBooleanFalse},
 		{operators.LessThan, xi(4), xi(3), types.XBooleanFalse},

excellent/tree.go

@@ -476,6 +476,23 @@ func (x *NumberLiteral) String() string {
 	return x.Value.Describe()
 }
 
+// ErrorLiteral is a literal error value
+type ErrorLiteral struct {
+	Err *types.XError
+}
+
+func (x *ErrorLiteral) Evaluate(env envs.Environment, scope *Scope, warnings *Warnings) types.XValue {
+	return x.Err
+}
+
+func (x *ErrorLiteral) Visit(v func(Expression)) {
+	v(x)
+}
+
+func (x *ErrorLiteral) String() string {
+	return "ERROR"
+}
+
 // BooleanLiteral is a literal bool
 type BooleanLiteral struct {
 	Value *types.XBoolean

excellent/types/number.go

@@ -15,6 +15,9 @@ import (
 // only parse numbers like 123 or 123.456 or .456
 var decimalRegexp = regexp.MustCompile(`^-?(([0-9]+)|([0-9]+\.[0-9]+)|(\.[0-9]+))$`)
 
+// MaxNumberDigits is the maximum number of significant digits in a number
+const MaxNumberDigits = 36
+
 func init() {
 	decimal.MarshalJSONWithoutQuotes = true
 }
@@ -33,19 +36,28 @@ type XNumber struct {
 	native decimal.Decimal
 }
 
-// NewXNumber creates a new XNumber
-func NewXNumber(value decimal.Decimal) *XNumber {
+// newXNumber creates a new XNumber without range checking - for use with known-safe values
+func newXNumber(value decimal.Decimal) *XNumber {
 	return &XNumber{native: value}
 }
 
+// NewXNumber creates a new XNumber from the given decimal value, returning an error if the value
+// is outside the range of values that can be persisted
+func NewXNumber(value decimal.Decimal) XValue {
+	if err := CheckDecimalRange(value); err != nil {
+		return NewXErrorf("number value out of range")
+	}
+	return newXNumber(value)
+}
+
 // NewXNumberFromInt creates a new XNumber from the given int
 func NewXNumberFromInt(value int) *XNumber {
-	return NewXNumber(decimal.New(int64(value), 0))
+	return newXNumber(decimal.New(int64(value), 0))
 }
 
 // NewXNumberFromInt64 creates a new XNumber from the given int
 func NewXNumberFromInt64(value int64) *XNumber {
-	return NewXNumber(decimal.New(value, 0))
+	return newXNumber(decimal.New(value, 0))
 }
 
 // RequireXNumberFromString creates a new XNumber from the given string or panics (used for tests)
@@ -133,9 +145,43 @@ func (x *XNumber) UnmarshalJSON(data []byte) error {
 }
 
 // XNumberZero is the zero number value
-var XNumberZero = NewXNumber(decimal.Zero)
+var XNumberZero = newXNumber(decimal.Zero)
 var _ XValue = XNumberZero
 
+// CheckDecimalRange checks that the given decimal value is within the range of values that can be
+// persisted to our database. It enforces two constraints:
+//
+//  1. The number of significant digits (excluding trailing zeros) must not exceed MaxNumberDigits (36).
+//  2. The magnitude (adjusted exponent) must be within ±100.
+//
+// Trailing zeros in the coefficient are not counted as significant, so a number like
+// 1234567895171680000000000000000000000000 (15 significant digits) is valid despite having 40 total digits.
+func CheckDecimalRange(d decimal.Decimal) error {
+	if d.IsZero() {
+		return nil
+	}
+
+	// count significant digits by removing trailing zeros from the coefficient
+	s := d.Coefficient().String()
+	s = strings.TrimLeft(s, "-")
+	s = strings.TrimRight(s, "0")
+	if len(s) > MaxNumberDigits {
+		return errors.New("number has too many digits")
+	}
+
+	adjExp := int64(d.Exponent()) + int64(d.NumDigits()) - 1
+	if adjExp > 100 || adjExp < -100 {
+		return errors.New("number value is out of permitted range")
+	}
+
+	return nil
+}
+
+// NewXNumberFromString parses a number from a string
+func NewXNumberFromString(s string) (*XNumber, error) {
+	return newXNumberFromString(s)
+}
+
 // parses a number from a string
 func newXNumberFromString(s string) (*XNumber, error) {
 	s = strings.TrimSpace(s)
@@ -147,7 +193,11 @@ func newXNumberFromString(s string) (*XNumber, error) {
 	// we can assume anything that matched our regex is parseable
 	d := decimal.RequireFromString(s)
 
-	return NewXNumber(d), nil
+	if err := CheckDecimalRange(d); err != nil {
+		return XNumberZero, err
+	}
+
+	return newXNumber(d), nil
 }
 
 // ToXNumber converts the given value to a number or returns an error if that isn't possible

excellent/types/number_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/nyaruka/gocommon/jsonx"
 	"github.com/nyaruka/goflow/envs"
 	"github.com/nyaruka/goflow/excellent/types"
+	"github.com/shopspring/decimal"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -48,6 +49,14 @@ func TestXNumber(t *testing.T) {
 	data, err := jsonx.Marshal(types.RequireXNumberFromString("23.45"))
 	assert.NoError(t, err)
 	assert.Equal(t, []byte(`23.45`), data)
+
+	// test NewXNumber range check
+	assert.False(t, types.IsXError(types.NewXNumber(decimal.New(123, 0))))
+	assert.True(t, types.IsXError(types.NewXNumber(decimal.New(1, 200))))             // magnitude too large
+	assert.True(t, types.IsXError(types.NewXNumber(decimal.New(1, -200))))            // magnitude too small
+	assert.False(t, types.IsXError(types.NewXNumber(decimal.New(1, 50))))             // within range
+	assert.False(t, types.IsXError(types.NewXNumber(decimal.New(1, -50))))            // within range
+	assert.False(t, types.IsXError(types.NewXNumber(decimal.RequireFromString("0")))) // zero always ok
 }
 
 func TestToXNumberAndInteger(t *testing.T) {
@@ -66,9 +75,11 @@ func TestToXNumberAndInteger(t *testing.T) {
 			"__default__": types.NewXNumberFromInt(123), // should use default
 			"foo":         types.NewXNumberFromInt(234),
 		}), types.NewXNumberFromInt(123), 123, false},
-		{types.NewXText("12345678901234567890"), types.RequireXNumberFromString("12345678901234567890"), 0, true}, // out of int range
-		{types.NewXText("1E100"), types.XNumberZero, 0, true},                                                     // scientific notation not allowed
-		{types.NewXText("1e100"), types.XNumberZero, 0, true},                                                     // scientific notation not allowed
+		{types.NewXText("12345678901234567890"), types.RequireXNumberFromString("12345678901234567890"), 0, true},                                 // out of int range
+		{types.NewXText("123456789012345678901234567890123456"), types.RequireXNumberFromString("123456789012345678901234567890123456"), 0, true}, // 36 digits, ok as number but out of int range
+		{types.NewXText("1234567890123456789012345678901234567"), types.XNumberZero, 0, true},                                                     // 37 digits, too many
+		{types.NewXText("1E100"), types.XNumberZero, 0, true},                                                                                     // scientific notation not allowed
+		{types.NewXText("1e100"), types.XNumberZero, 0, true},                                                                                     // scientific notation not allowed
 		{types.NewXText("234."), types.XNumberZero, 0, true},
 		{types.NewXText("+1800567890"), types.XNumberZero, 0, true},
 	}
@@ -89,6 +100,37 @@ func TestToXNumberAndInteger(t *testing.T) {
 	}
 }
 
+func TestCheckDecimalRange(t *testing.T) {
+	tests := []struct {
+		value   decimal.Decimal
+		isError bool
+	}{
+		{decimal.Zero, false},
+		{decimal.New(1, 0), false},
+		{decimal.New(-1, 0), false},
+		{decimal.New(123, 0), false},
+		{decimal.RequireFromString("123456789012345678901234567890123456"), false},                  // 36 significant digits - ok
+		{decimal.RequireFromString("1234567890123456789012345678901234567"), true},                 // 37 significant digits - too many
+		{decimal.RequireFromString("-1234567890123456789012345678901234567"), true},                // negative 37 significant digits
+		{decimal.RequireFromString("1234567895171680000000000000000000000000"), false},             // 40 digits but only 15 significant - ok
+		{decimal.RequireFromString("12345678901234567890123456789012345670000000"), true},          // 37 significant digits with trailing zeros
+		{decimal.RequireFromString("0.000000000000000000000000000000000001"), false},
+		{decimal.New(1, 100), false}, // 1E100 - ok magnitude
+		{decimal.New(1, 200), true},  // 1E200 - too large magnitude
+		{decimal.New(1, -100), false},
+		{decimal.New(1, -200), true}, // 1E-200 - too small magnitude
+	}
+
+	for _, tc := range tests {
+		err := types.CheckDecimalRange(tc.value)
+		if tc.isError {
+			assert.Error(t, err, "expected error for %s", tc.value)
+		} else {
+			assert.NoError(t, err, "unexpected error for %s", tc.value)
+		}
+	}
+}
+
 func TestFormatCustom(t *testing.T) {
 	fmtTests := []struct {
 		input       *types.XNumber

excellent/visitor.go

@@ -218,7 +218,11 @@ func (v *visitor) VisitTextLiteral(ctx *gen.TextLiteralContext) any {
 
 // VisitNumberLiteral deals with numbers like 123 or 1.5
 func (v *visitor) VisitNumberLiteral(ctx *gen.NumberLiteralContext) any {
-	return &NumberLiteral{Value: types.RequireXNumberFromString(ctx.GetText())}
+	num, err := types.NewXNumberFromString(ctx.GetText())
+	if err != nil {
+		return &ErrorLiteral{Err: types.NewXErrorf("number %s is out of range", ctx.GetText())}
+	}
+	return &NumberLiteral{Value: num}
 }
 
 // VisitTrue deals with the `true` reserved word

flows/routers/cases/utils.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/nyaruka/goflow/envs"
+	"github.com/nyaruka/goflow/excellent/types"
 
 	"github.com/shopspring/decimal"
 )
@@ -67,5 +68,14 @@ func ParseDecimal(val string, format *envs.NumberFormat) (decimal.Decimal, error
 	// replace non-Arabic (0-9) numerals with their equivalents
 	cleaned = strings.Map(numeralMapper, cleaned)
 
-	return decimal.NewFromString(cleaned)
+	d, err := decimal.NewFromString(cleaned)
+	if err != nil {
+		return d, err
+	}
+
+	if err := types.CheckDecimalRange(d); err != nil {
+		return decimal.Zero, err
+	}
+
+	return d, nil
 }

flows/routers/cases/utils_test.go

@@ -46,4 +46,8 @@ func TestParseDecimal(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, test.expected, val, "parse decimal failed for input '%s'", test.input)
 	}
+
+	// test that oversized numbers are rejected
+	_, err := cases.ParseDecimal("1234567890123456789012345678901234567", envs.DefaultNumberFormat)
+	assert.EqualError(t, err, "number has too many digits")
 }