Skip to content

Commit 258fd81

Browse files
author
suoyi
committed
fix: reject values at or above 2^63 in int parameter parsing
toInt's upper bound compares against math.MaxInt, which is 2^(strconv.IntSize-1) - 1 and cannot be represented as a float64: on 64-bit targets it rounds up to exactly 2^63, so a value that parses to 2^63 is not greater than the bound and passed the check. It then reached int(f), and converting an out-of-range float64 to int is implementation-defined - MinInt on amd64 - so the parameter was accepted and a large negative number was sent to the API instead of failing the range check. That covered every caller of RequiredInt, OptionalIntParam, OptionalIntParamWithDefault and OptionalPaginationParams, including the string "9223372036854775807", which ParseFloat rounds to 2^63. Compare against math.Ldexp(1, strconv.IntSize-1) instead. It is the exact power of two on both 32- and 64-bit targets, so the bound holds everywhere and the conversion is never reached with an out-of-range value - unlike inspecting the result of the conversion, which only catches the overflow on targets that do not saturate.
1 parent c59e8c1 commit 258fd81

2 files changed

Lines changed: 36 additions & 1 deletion

File tree

‎pkg/github/params.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,14 @@ func toInt(val any) (int, error) {
8383
if f != math.Trunc(f) {
8484
return 0, fmt.Errorf("non-integer numeric value: %v", f)
8585
}
86-
if f > math.MaxInt || f < math.MinInt {
86+
// Compare before converting, against the exact power of two. math.MaxInt is
87+
// 2^(strconv.IntSize-1) - 1, which a float64 cannot represent: on 64-bit
88+
// targets it rounds up to exactly 2^63, so comparing against it lets 2^63
89+
// through. Converting an out-of-range float64 to int is implementation-defined
90+
// - MinInt on amd64, MaxInt on the targets whose conversion saturates - so the
91+
// bound has to reject the value before the conversion rather than inspect the
92+
// result of it. math.Ldexp gives the exact bound on every target.
93+
if limit := math.Ldexp(1, strconv.IntSize-1); f >= limit || f < -limit {
8794
return 0, fmt.Errorf("numeric value out of int range: %v", f)
8895
}
8996
return int(f), nil

‎pkg/github/params_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,34 @@ func Test_OptionalIntParam(t *testing.T) {
376376
expected: 0,
377377
expectError: true,
378378
},
379+
{
380+
name: "2^63 string parameter",
381+
params: map[string]any{"count": "9223372036854775808"},
382+
paramName: "count",
383+
expected: 0,
384+
expectError: true,
385+
},
386+
{
387+
name: "MaxInt64 string parameter rounds up to 2^63",
388+
params: map[string]any{"count": "9223372036854775807"},
389+
paramName: "count",
390+
expected: 0,
391+
expectError: true,
392+
},
393+
{
394+
name: "2^63 float64 parameter",
395+
params: map[string]any{"count": math.Pow(2, 63)},
396+
paramName: "count",
397+
expected: 0,
398+
expectError: true,
399+
},
400+
{
401+
name: "MaxInt32 float64 parameter",
402+
params: map[string]any{"count": float64(math.MaxInt32)},
403+
paramName: "count",
404+
expected: math.MaxInt32,
405+
expectError: false,
406+
},
379407
}
380408

381409
for _, tc := range tests {

0 commit comments

Comments
 (0)