Skip to content

Fix #46: size the rejection-sampling mask without float overflow - #95

Open
corgab wants to merge 2 commits into
mainfrom
fix/46-integer-range-overflow
Open

Fix #46: size the rejection-sampling mask without float overflow#95
corgab wants to merge 2 commits into
mainfrom
fix/46-integer-range-overflow

Conversation

@corgab

@corgab corgab commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

EntropyGenerator::integer() sized its rejection-sampling chunks with (int) ceil(log($range + 1, 2)) and masked them with (1 << $bitsNeeded) - 1. Two things break near the top of the 64-bit range:

  • (1 << 63) - 1 overflows to a negative float, so integer(0, PHP_INT_MAX) used a corrupted mask, as the issue reports.
  • The float logarithm loses precision above 2^53: for range = 2^62, 2^62 + 1 rounds to 2^62 as a double, log2 gives exactly 62, and only 62 bits were drawn, so the top value of the range could never be produced. Verified: (int) ceil(log(2**62 + 1, 2)) is 62 while the range needs 63 bits. The issue's exhaustive check missed this because it covered ranges of the form 2^k - 1.

Plan

Derive the bit count with integer arithmetic, drop the mask (every chunk is exactly $bitsNeeded binary digits, so bindec() is already in range), and reject the one input the algorithm cannot serve: a span that overflows the subtraction, which would need a 64-bit chunk that bindec() returns as a float.

Changes

  • EntropyGenerator::integer(): the overflow is detected before subtracting ($min < 0 && $max > PHP_INT_MAX + $min), which keeps $range an int for PHPStan and avoids relying on float promotion; $bitsNeeded = strlen(decbin($range)); the mask is gone. Docblock states the accepted bounds.
  • README entropy section: one sentence on the accepted span.

The new guard throws \InvalidArgumentException, like the two argument checks already in this class (min > max, bits < 1): these are caller mistakes on plain integer arguments, not package failures, so they stay outside the AetherException hierarchy on purpose.

No config change, no new dependencies, no change for ordinary ranges (the bit length is identical below 2^53 and the mask never altered a value).

Tests

EntropyGeneratorTest, under a "Wide ranges" banner: integer(0, PHP_INT_MAX) returns PHP_INT_MAX from an all-ones chunk; integer(0, 2 ** 62) returns 2 ** 62 from a chunk of 1 followed by zeros (fails on main, which returns 2 ** 61); integer(PHP_INT_MIN, PHP_INT_MAX) throws before touching the device. The guard was also checked by hand on (PHP_INT_MIN, 0), (PHP_INT_MIN, -1), (0, PHP_INT_MAX) and (-1, PHP_INT_MAX).

Local: pint passed, pest 794 tests passed.

Closes #46

integer() derived the bit count from ceil(log(range + 1, 2)) and the mask
from 1 << bits. Both break near the top of the 64-bit range: the float
logarithm under-counts for ranges such as 2^62, so the top value could
never be drawn, and 1 << 63 overflows to a negative float, so
integer(0, PHP_INT_MAX) used a corrupted mask. The bit length now comes
from decbin(), which is exact, the 63-bit mask is PHP_INT_MAX itself, and
a span that overflows the subtraction is rejected with a clear
InvalidArgumentException instead of reaching bindec() as a float.

Closes #46
… mask

PHPStan types int - int as int, so a post-hoc is_int() check reads as
always true; the guard now compares the bounds against PHP_INT_MAX + $min
before subtracting, which also removes the reliance on float promotion.
The mask was a no-op since every chunk is exactly $bitsNeeded digits, so
bindec() alone yields the value. The new tests sit under their own banner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[entropy] EntropyGenerator::integer() mask computation overflows for ranges approaching PHP_INT_MAX

1 participant