Skip to content

Commit 4f250f5

Browse files
authored
fix: reject integer parameters at or above 2^63 (#3650)
* fix: reject integer parameters at or above 2^63 math.MaxInt64 is not representable as a float64 and rounds up to 2^63, so range checks against it let 2^63 through to an implementation-defined float-to-int conversion (MinInt on amd64, MaxInt on arm64). Add inSignedIntRange, which compares against the exact power-of-two bound before converting, and use it in toInt, toInt64 and the security and typed-read argument normalizers. Fixes #3489 * Remove inSignedIntRange doc comment
1 parent 3366807 commit 4f250f5

6 files changed

Lines changed: 131 additions & 8 deletions

File tree

‎pkg/github/params.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ func isAcceptedError(err error) bool {
6060
return errors.As(err, &acceptedError)
6161
}
6262

63+
func inSignedIntRange(f float64, bitSize int) bool {
64+
limit := math.Ldexp(1, bitSize-1)
65+
return f >= -limit && f < limit
66+
}
67+
6368
// toInt converts a value to int, handling both float64 and string representations.
6469
// Some MCP clients send numeric values as strings. It rejects NaN, ±Inf,
6570
// fractional values, and values outside the int range.
@@ -83,15 +88,15 @@ func toInt(val any) (int, error) {
8388
if f != math.Trunc(f) {
8489
return 0, fmt.Errorf("non-integer numeric value: %v", f)
8590
}
86-
if f > math.MaxInt || f < math.MinInt {
91+
if !inSignedIntRange(f, strconv.IntSize) {
8792
return 0, fmt.Errorf("numeric value out of int range: %v", f)
8893
}
8994
return int(f), nil
9095
}
9196

9297
// toInt64 converts a value to int64, handling both float64 and string representations.
9398
// Some MCP clients send numeric values as strings. It rejects NaN, ±Inf,
94-
// fractional values, and values that lose precision in the float64→int64 conversion.
99+
// fractional values, and values outside the int64 range.
95100
func toInt64(val any) (int64, error) {
96101
var f float64
97102
switch v := val.(type) {
@@ -112,12 +117,10 @@ func toInt64(val any) (int64, error) {
112117
if f != math.Trunc(f) {
113118
return 0, fmt.Errorf("non-integer numeric value: %v", f)
114119
}
115-
result := int64(f)
116-
// Check round-trip to detect precision loss for large int64 values
117-
if float64(result) != f {
120+
if !inSignedIntRange(f, 64) {
118121
return 0, fmt.Errorf("numeric value %v is too large to fit in int64", f)
119122
}
120-
return result, nil
123+
return int64(f), nil
121124
}
122125

123126
// RequiredParam is a helper function that can be used to fetch a requested parameter from the request.

‎pkg/github/params_test.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,65 @@ func TestOptionalNullableStringParam(t *testing.T) {
184184
}
185185
}
186186

187+
func Test_inSignedIntRange(t *testing.T) {
188+
tests := []struct {
189+
name string
190+
value float64
191+
bitSize int
192+
want bool
193+
}{
194+
{"int64 min", math.MinInt64, 64, true},
195+
{"largest float64 below 2^63", math.Nextafter(math.Ldexp(1, 63), 0), 64, true},
196+
{"2^63", math.Ldexp(1, 63), 64, false},
197+
{"MaxInt64 rounds up to 2^63", float64(math.MaxInt64), 64, false},
198+
{"below int64 min", math.Nextafter(math.MinInt64, math.Inf(-1)), 64, false},
199+
{"int32 max", math.MaxInt32, 32, true},
200+
{"2^31", math.MaxInt32 + 1, 32, false},
201+
{"int32 min", math.MinInt32, 32, true},
202+
{"below int32 min", math.MinInt32 - 1, 32, false},
203+
{"NaN", math.NaN(), 64, false},
204+
{"positive infinity", math.Inf(1), 64, false},
205+
{"negative infinity", math.Inf(-1), 64, false},
206+
}
207+
208+
for _, tc := range tests {
209+
t.Run(tc.name, func(t *testing.T) {
210+
assert.Equal(t, tc.want, inSignedIntRange(tc.value, tc.bitSize))
211+
})
212+
}
213+
}
214+
215+
func Test_RequiredBigInt(t *testing.T) {
216+
tests := []struct {
217+
name string
218+
value any
219+
expected int64
220+
expectedErr string
221+
}{
222+
{"valid number", float64(42), 42, ""},
223+
{"valid string", "42", 42, ""},
224+
{"MinInt64 string", "-9223372036854775808", math.MinInt64, ""},
225+
{"largest float64 below 2^63", math.Nextafter(math.Ldexp(1, 63), 0), 9223372036854774784, ""},
226+
{"2^63 string", "9223372036854775808", 0, "too large to fit in int64"},
227+
{"MaxInt64 string rounds up to 2^63", "9223372036854775807", 0, "too large to fit in int64"},
228+
{"2^63 float64", math.Ldexp(1, 63), 0, "too large to fit in int64"},
229+
{"below MinInt64", "-9223372036854777856", 0, "too large to fit in int64"},
230+
}
231+
232+
for _, tc := range tests {
233+
t.Run(tc.name, func(t *testing.T) {
234+
result, err := RequiredBigInt(map[string]any{"id": tc.value}, "id")
235+
236+
if tc.expectedErr != "" {
237+
assert.ErrorContains(t, err, tc.expectedErr)
238+
return
239+
}
240+
require.NoError(t, err)
241+
assert.Equal(t, tc.expected, result)
242+
})
243+
}
244+
}
245+
187246
func Test_RequiredInt(t *testing.T) {
188247
tests := []struct {
189248
name string
@@ -376,6 +435,34 @@ func Test_OptionalIntParam(t *testing.T) {
376435
expected: 0,
377436
expectError: true,
378437
},
438+
{
439+
name: "2^63 string parameter",
440+
params: map[string]any{"count": "9223372036854775808"},
441+
paramName: "count",
442+
expected: 0,
443+
expectError: true,
444+
},
445+
{
446+
name: "MaxInt64 string parameter rounds up to 2^63",
447+
params: map[string]any{"count": "9223372036854775807"},
448+
paramName: "count",
449+
expected: 0,
450+
expectError: true,
451+
},
452+
{
453+
name: "2^63 float64 parameter",
454+
params: map[string]any{"count": math.Ldexp(1, 63)},
455+
paramName: "count",
456+
expected: 0,
457+
expectError: true,
458+
},
459+
{
460+
name: "MinInt float64 parameter",
461+
params: map[string]any{"count": float64(math.MinInt)},
462+
paramName: "count",
463+
expected: math.MinInt,
464+
expectError: false,
465+
},
379466
}
380467

381468
for _, tc := range tests {

‎pkg/github/security_outputs.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ func normalizeSecurityIntegerArguments(fields ...string) inventory.InputNormaliz
301301
}
302302
parsed, err := strconv.ParseFloat(number, 64)
303303
if err != nil || math.IsNaN(parsed) || math.IsInf(parsed, 0) ||
304-
parsed != math.Trunc(parsed) || parsed > float64(math.MaxInt) || parsed < float64(math.MinInt) {
304+
parsed != math.Trunc(parsed) || !inSignedIntRange(parsed, strconv.IntSize) {
305305
return nil, fmt.Errorf("parameter %s is not a valid number", field)
306306
}
307307
arguments[field], err = json.Marshal(int(parsed))

‎pkg/github/typed_read_normalizers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func normalizeTypedReadArguments(uppercaseFields []string, preserveZeroPage bool
4242
}
4343
if text, ok := number.(string); ok {
4444
parsed, err := strconv.ParseFloat(text, 64)
45-
if err != nil || math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed != math.Trunc(parsed) || parsed > float64(math.MaxInt) || parsed < float64(math.MinInt) {
45+
if err != nil || math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed != math.Trunc(parsed) || !inSignedIntRange(parsed, strconv.IntSize) {
4646
return nil, fmt.Errorf("parameter %s is not a valid number", field)
4747
}
4848
number = int(parsed)

‎pkg/github/typed_search_outputs_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,20 @@ func TestNormalizeTypedReadArgumentsPreservesLegacyValues(t *testing.T) {
377377
assert.JSONEq(t, `{"state":"OPEN","orderBy":"UPDATED_AT","direction":"ASC","perPage":25,"page":null}`, string(normalized))
378378
}
379379

380+
func TestNormalizeTypedReadArgumentsRejectsOutOfRangeIntegers(t *testing.T) {
381+
normalize := normalizeTypedReadArguments(nil, false)
382+
for _, input := range []string{
383+
`{"page":"9223372036854775808"}`,
384+
`{"perPage":"9223372036854775807"}`,
385+
`{"page":"-9223372036854777856"}`,
386+
} {
387+
t.Run(input, func(t *testing.T) {
388+
_, err := normalize(json.RawMessage(input))
389+
require.ErrorContains(t, err, "is not a valid number")
390+
})
391+
}
392+
}
393+
380394
func mustMarshalJSON(t *testing.T, value any) string {
381395
t.Helper()
382396
encoded, err := json.Marshal(value)

‎pkg/github/typed_security_outputs_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,3 +390,22 @@ func TestSecurityOutputDTOs(t *testing.T) {
390390
check(typ)
391391
}
392392
}
393+
394+
func TestNormalizeSecurityIntegerArgumentsRejectsOutOfRangeIntegers(t *testing.T) {
395+
normalize := normalizeSecurityIntegerArguments("alertNumber")
396+
for _, input := range []string{
397+
`{"alertNumber":9223372036854775807}`,
398+
`{"alertNumber":"9223372036854775807"}`,
399+
`{"alertNumber":"9223372036854775808"}`,
400+
`{"alertNumber":"-9223372036854777856"}`,
401+
} {
402+
t.Run(input, func(t *testing.T) {
403+
_, err := normalize(json.RawMessage(input))
404+
require.EqualError(t, err, "parameter alertNumber is not a valid number")
405+
})
406+
}
407+
408+
normalized, err := normalize(json.RawMessage(`{"alertNumber":"2147483647"}`))
409+
require.NoError(t, err)
410+
assert.JSONEq(t, `{"alertNumber":2147483647}`, string(normalized))
411+
}

0 commit comments

Comments
 (0)