Skip to content

Commit 89fb685

Browse files
committed
fix: edge case with ~uint64 weight >= platform maxInt
Exposed during fuzzing, possible edge (very edge!) case where when using a Chooser[any, ~uint64], if a single Choice weight exceeds system maxInt, then when it gets converted to system int type it no longer triggers the the check for exceeding maxInt in the running total, resulting in a Chooser that can panic on Pick.
1 parent 987c3d8 commit 89fb685

File tree

3 files changed

+45
-5
lines changed

3 files changed

+45
-5
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
go test fuzz v1
2+
[]byte("0000000\x8f00000000")

weightedrand.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,16 @@ func NewChooser[T any, W integer](choices ...Choice[T, W]) (*Chooser[T, W], erro
4848
totals := make([]int, len(choices))
4949
runningTotal := 0
5050
for i, c := range choices {
51-
weight := int(c.Weight)
52-
if weight < 0 {
51+
if c.Weight < 0 {
5352
continue // ignore negative weights, can never be picked
5453
}
5554

55+
// case of single ~uint64 or similar value that exceeds maxInt on its own
56+
if uint64(c.Weight) >= maxInt {
57+
return nil, errWeightOverflow
58+
}
59+
60+
weight := int(c.Weight) // convert weight to int for internal counter usage
5661
if (maxInt - runningTotal) <= weight {
5762
return nil, errWeightOverflow
5863
}
@@ -68,8 +73,9 @@ func NewChooser[T any, W integer](choices ...Choice[T, W]) (*Chooser[T, W], erro
6873
}
6974

7075
const (
71-
intSize = 32 << (^uint(0) >> 63) // cf. strconv.IntSize
72-
maxInt = 1<<(intSize-1) - 1
76+
intSize = 32 << (^uint(0) >> 63) // cf. strconv.IntSize
77+
maxInt = 1<<(intSize-1) - 1
78+
maxUint64 = 1<<64 - 1
7379
)
7480

7581
// Possible errors returned by NewChooser, preventing the creation of a Chooser

weightedrand_test.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,42 @@ func TestNewChooser(t *testing.T) {
8181
}
8282
for _, tt := range tests {
8383
t.Run(tt.name, func(t *testing.T) {
84-
_, err := NewChooser(tt.cs...)
84+
c, err := NewChooser(tt.cs...)
8585
if err != tt.wantErr {
8686
t.Errorf("NewChooser() error = %v, wantErr %v", err, tt.wantErr)
8787
}
88+
89+
if err == nil { // run a few Picks to make sure there are no panics
90+
for i := 0; i < 10; i++ {
91+
_ = c.Pick()
92+
}
93+
}
94+
})
95+
}
96+
97+
u64tests := []struct {
98+
name string
99+
cs []Choice[rune, uint64]
100+
wantErr error
101+
}{
102+
{
103+
name: "weight overflow from single uint64 exceeding system maxInt",
104+
cs: []Choice[rune, uint64]{{Item: 'a', Weight: maxInt + 1}},
105+
wantErr: errWeightOverflow,
106+
},
107+
}
108+
for _, tt := range u64tests {
109+
t.Run(tt.name, func(t *testing.T) {
110+
c, err := NewChooser(tt.cs...)
111+
if err != tt.wantErr {
112+
t.Errorf("NewChooser() error = %v, wantErr %v", err, tt.wantErr)
113+
}
114+
115+
if err == nil { // run a few Picks to make sure there are no panics
116+
for i := 0; i < 10; i++ {
117+
_ = c.Pick()
118+
}
119+
}
88120
})
89121
}
90122
}

0 commit comments

Comments
 (0)