diff --git a/base_lexer.go b/base_lexer.go index 0261c37..79c1546 100644 --- a/base_lexer.go +++ b/base_lexer.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "math" "slices" "strconv" ) @@ -107,30 +108,16 @@ func (l *baseLexer) readUint64Field() (i uint64, err error) { //nolint:cyclop break } - switch ch { - case '0': - i *= 10 - case '1': - i = i*10 + 1 - case '2': - i = i*10 + 2 - case '3': - i = i*10 + 3 - case '4': - i = i*10 + 4 - case '5': - i = i*10 + 5 - case '6': - i = i*10 + 6 - case '7': - i = i*10 + 7 - case '8': - i = i*10 + 8 - case '9': - i = i*10 + 9 - default: + if ch < '0' || ch > '9' { return i, l.syntaxError() } + + digit := uint64(ch - '0') + // Check for overflow and return syntax error if it would occur. + if i > (math.MaxUint64-digit)/10 { + return i, l.syntaxError() + } + i = i*10 + digit } return i, nil diff --git a/base_lexer_test.go b/base_lexer_test.go index 0ac1daa..2aa9c34 100644 --- a/base_lexer_test.go +++ b/base_lexer_test.go @@ -31,6 +31,14 @@ func TestLexer(t *testing.T) { assert.Error(t, err) }) + t.Run("uint64 overflow returns syntax error", func(t *testing.T) { + // 18446744073709551616 == 2^64, wraps to 0 without overflow check + l := &baseLexer{value: "18446744073709551616"} + _, err := l.readUint64Field() + var se syntaxError + assert.ErrorAs(t, err, &se, "overflow should return a syntaxError") + }) + t.Run("many fields", func(t *testing.T) { lex := &baseLexer{value: "aaa 123\nf1 f2\nlast"} diff --git a/extmap.go b/extmap.go index fdd1d81..50defef 100644 --- a/extmap.go +++ b/extmap.go @@ -52,12 +52,12 @@ func (e *ExtMap) Unmarshal(raw string) error { valdir := strings.Split(fields[0], "/") value, err := strconv.ParseInt(valdir[0], 10, 64) - if (value < 1) || (value > 246) { - return fmt.Errorf("%w: %v -- extmap key must be in the range 1-256", errSyntaxError, valdir[0]) - } if err != nil { return fmt.Errorf("%w: %v", errSyntaxError, valdir[0]) } + if (value < 1) || (value > 256) { + return fmt.Errorf("%w: %v -- extmap key must be in the range 1-256", errSyntaxError, valdir[0]) + } var direction Direction if len(valdir) == 2 { diff --git a/extmap_test.go b/extmap_test.go index 086b787..3c82d0a 100644 --- a/extmap_test.go +++ b/extmap_test.go @@ -94,3 +94,27 @@ func TestExtMap_Unmarshal_Error_URLParse(t *testing.T) { err := em.Unmarshal("extmap:1 http://example.com/%zz") assert.Error(t, err) } + +func TestExtMap_Unmarshal_ValueBounds(t *testing.T) { + t.Run("value 1 accepted (lower bound)", func(t *testing.T) { + var em ExtMap + assert.NoError(t, em.Unmarshal("extmap:1 http://example.com")) + assert.Equal(t, 1, em.Value) + }) + + t.Run("value 0 rejected (below lower bound)", func(t *testing.T) { + var em ExtMap + assert.ErrorIs(t, em.Unmarshal("extmap:0 http://example.com"), errSyntaxError) + }) + + t.Run("value 256 accepted (upper bound)", func(t *testing.T) { + var em ExtMap + assert.NoError(t, em.Unmarshal("extmap:256 http://example.com")) + assert.Equal(t, 256, em.Value) + }) + + t.Run("value 257 rejected (above upper bound)", func(t *testing.T) { + var em ExtMap + assert.ErrorIs(t, em.Unmarshal("extmap:257 http://example.com"), errSyntaxError) + }) +} diff --git a/unmarshal.go b/unmarshal.go index 2f011a8..9271b4a 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -6,6 +6,7 @@ package sdp import ( "errors" "fmt" + "math" "net/url" "strconv" "strings" @@ -998,18 +999,23 @@ func parseTimeUnits(value string) (num int64, err error) { if len(value) == 0 { return 0, fmt.Errorf("%w `%v`", errSDPInvalidValue, value) } - k := timeShorthand(value[len(value)-1]) - if k > 0 { + factor := timeShorthand(value[len(value)-1]) + if factor > 0 { num, err = strconv.ParseInt(value[:len(value)-1], 10, 64) } else { - k = 1 + factor = 1 num, err = strconv.ParseInt(value, 10, 64) } if err != nil { return 0, fmt.Errorf("%w `%v`", errSDPInvalidValue, value) } - return num * k, nil + // Check for overflow and return error if it would occur. + if factor > 1 && (num > math.MaxInt64/factor || num < math.MinInt64/factor) { + return 0, fmt.Errorf("%w `%v`", errSDPInvalidValue, value) + } + + return num * factor, nil } func timeShorthand(b byte) int64 { diff --git a/unmarshal_test.go b/unmarshal_test.go index 2e0ada7..d10fe8d 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -1909,3 +1909,18 @@ func TestTimeShorthand_MinutesAndSeconds(t *testing.T) { assert.Equal(t, int64(1), timeShorthand('s')) }) } + +func TestParseTimeUnits_Overflow(t *testing.T) { + // math.MaxInt64 = 9223372036854775807; multiplied by 3600 (h) overflows int64 + _, err := parseTimeUnits("9223372036854775807h") + assert.Error(t, err, "positive overflow with 'h' shorthand should return error") + + // math.MinInt64 = -9223372036854775808; multiplied by 3600 (h) overflows int64 + _, err = parseTimeUnits("-9223372036854775808h") + assert.Error(t, err, "negative overflow with 'h' shorthand should return error") + + // Largest value that does NOT overflow with 'd' (86400): math.MaxInt64/86400 = 106751991167300 + v, err := parseTimeUnits("106751991167300d") + assert.NoError(t, err) + assert.Equal(t, int64(106751991167300)*86400, v) +}