fix: allow increment of 1 in Monotonic entropy for io.Reader sources

MonotonicEntropy.random rejected a validly drawn value of 0 in its
rejection-sampling loop, making an increment of exactly 1 unreachable
for entropy sources without a fast Int63n path (e.g. crypto/rand.Reader).
For inc == 2 this made the increment fully deterministic (always 2),
contradicting the documented "random number between 1 and inc inclusive"
contract. Restructure the loop to match the crypto/rand.Int pattern it is
adapted from, so 0 is a valid draw and the result is correctly uniform
over [1, inc].
This commit is contained in:
dualfroz committed 2026-09-04 04:36:21 +02:00
1 parent e2c3f62f18
commit e11375a882
2 files changed
+70 -4

No files matched your search

+10 -4
View File
@@ -641,7 +641,7 @@ func (m *MonotonicEntropy) increment() error {
return nil
}
// random returns a uniform random value in [1, m.inc), reading entropy
// random returns a uniform random value in [1, m.inc], reading entropy
// from m.Reader. When m.inc == 0 || m.inc == 1, it returns 1.
// Adapted from: https://golang.org/pkg/crypto/rand/#Int
func (m *MonotonicEntropy) random() (inc uint64, err error) {
@@ -651,7 +651,7 @@ func (m *MonotonicEntropy) random() (inc uint64, err error) {
// Fast path for using a underlying rand.Rand directly.
if m.rng != nil {
// Range: [1, m.inc)
// Range: [1, m.inc]
return 1 + uint64(m.rng.Int63n(int64(m.inc))), nil
}
@@ -667,7 +667,7 @@ func (m *MonotonicEntropy) random() (inc uint64, err error) {
msbitLen = 8
}
for inc == 0 || inc >= m.inc {
for {
if _, err = io.ReadFull(m.Reader, m.rand[:byteLen]); err != nil {
return 0, err
}
@@ -688,9 +688,15 @@ func (m *MonotonicEntropy) random() (inc uint64, err error) {
case 5, 6, 7, 8:
inc = uint64(binary.LittleEndian.Uint64(m.rand[:8]))
}
// inc is uniform over [0, 2^byteLen*8), which may exceed m.inc; if
// so, redraw to avoid bias toward the low end of [0, m.inc).
if inc < m.inc {
break
}
}
// Range: [1, m.inc)
// Range: [1, m.inc]
return 1 + inc, nil
}
+60
View File
@@ -19,6 +19,7 @@ import (
"fmt"
"io"
"math"
"math/big"
"math/rand"
"strings"
"testing"
@@ -629,6 +630,65 @@ func TestMonotonic(t *testing.T) {
}
}
// TestMonotonicIncrementRange is a regression test for a bug where
// MonotonicEntropy.random incorrectly rejected a masked draw of 0, making an
// increment of 1 unreachable and, for inc == 2, making the increment fully
// deterministic (always 2). It drives Monotonic with a reader that yields an
// exact, controlled byte sequence so the test result does not depend on
// chance draws from a real entropy source.
//
// With inc == 2, random() reads one byte at a time and keeps only its low 2
// bits (see MonotonicEntropy.random). A masked value of 0 must yield an
// increment of 1, a masked value of 1 must yield an increment of 2, and
// masked values of 2 or 3 are out of range and must be rejected and redrawn.
func TestMonotonicIncrementRange(t *testing.T) {
t.Parallel()
// The first 10 bytes seed the initial entropy for the first ULID; their
// bit pattern doesn't matter to random(), only that the resulting value
// is non-zero and far from overflow. The remaining bytes are consumed by
// random(), one at a time (or more, on rejection), to compute each
// subsequent increment.
seed := append([]byte{0x01}, bytes.Repeat([]byte{0x00}, 9)...)
draws := []byte{0x00, 0x01, 0x02, 0x03, 0x00}
reader := bytes.NewReader(append(seed, draws...))
entropy := ulid.Monotonic(reader, 2)
prev, err := ulid.New(123, entropy)
if err != nil {
t.Fatal(err)
}
// draw byte 0x00 -> masked 0 -> accepted -> increment 1
// draw byte 0x01 -> masked 1 -> accepted -> increment 2
// draw byte 0x02 -> masked 2 -> rejected (>= inc), redraw
// draw byte 0x03 -> masked 3 -> rejected (>= inc), redraw
// draw byte 0x00 -> masked 0 -> accepted -> increment 1
wantDeltas := []uint64{1, 2, 1}
for i, want := range wantDeltas {
next, err := ulid.New(123, entropy)
if err != nil {
t.Fatalf("draw %d: %v", i, err)
}
if prev.Compare(next) >= 0 {
t.Fatalf("draw %d: monotonicity violated: prev=%v next=%v", i, prev, next)
}
delta := new(big.Int).Sub(
new(big.Int).SetBytes(next.Entropy()),
new(big.Int).SetBytes(prev.Entropy()),
).Uint64()
if delta != want {
t.Fatalf("draw %d: delta = %d, want %d", i, delta, want)
}
prev = next
}
}
func TestMonotonicOverflow(t *testing.T) {
t.Parallel()