Skip to content

Fix GH-21731: Random\Engine\Xoshiro256StarStar::__unserialize() accepts all-zero state#21732

Merged
TimWolla merged 1 commit intophp:PHP-8.4from
iliaal:fix/gh-21731-xoshiro-unserialize-zero-state
Apr 12, 2026
Merged

Fix GH-21731: Random\Engine\Xoshiro256StarStar::__unserialize() accepts all-zero state#21732
TimWolla merged 1 commit intophp:PHP-8.4from
iliaal:fix/gh-21731-xoshiro-unserialize-zero-state

Conversation

@iliaal
Copy link
Copy Markdown
Contributor

@iliaal iliaal commented Apr 12, 2026

Fixes #21731.

Random\Engine\Xoshiro256StarStar::__construct() rejects a seed that would leave the internal state all zero, because xoshiro256** with zero state returns 0 on every call forever. The unserialize callback didn't check the same invariant. A caller feeding a crafted serialized payload through __unserialize() ended up with a live engine that returned 0 from every operation.

Match the constructor: reject the all-zero state from unserialize too. The Mt19937-aliased __unserialize() wrapper maps the false return into the standard "Invalid serialization data for ... object" exception, so the wrapper needs no changes.

@iliaal iliaal force-pushed the fix/gh-21731-xoshiro-unserialize-zero-state branch from cd7de3e to de3fc73 Compare April 12, 2026 15:38
@iliaal iliaal requested a review from TimWolla April 12, 2026 15:40
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears you accidentally dropped the NEWS change during the rebase. Given this is a behavioral change / proper bugfix, it should be there. Do you want to add it back in, or shall I?

…cepts all-zero state

The constructor rejects a seed that would leave the internal state
all zero, because xoshiro256** with zero state produces 0 on every
call forever. The unserialize callback didn't check the same
invariant. A caller feeding a crafted serialized payload through
__unserialize() ended up with a live engine that returned 0 from
every operation.

Match the constructor: reject the all-zero state from the unserialize
callback too. The Mt19937-aliased __unserialize() wrapper turns the
false return into the standard "Invalid serialization data" exception.

Closes phpGH-21731
@iliaal iliaal force-pushed the fix/gh-21731-xoshiro-unserialize-zero-state branch from de3fc73 to 8ea43b9 Compare April 12, 2026 16:54
@iliaal
Copy link
Copy Markdown
Contributor Author

iliaal commented Apr 12, 2026

It appears you accidentally dropped the NEWS change during the rebase. Given this is a behavioral change / proper bugfix, it should be there. Do you want to add it back in, or shall I?

Added it back, I went a little heavy on removals 😓

@TimWolla TimWolla merged commit 1a428e5 into php:PHP-8.4 Apr 12, 2026
19 checks passed
TimWolla added a commit that referenced this pull request Apr 12, 2026
* PHP-8.4:
  Fix GH-21731: Random\Engine\Xoshiro256StarStar::__unserialize() accepts all-zero state (#21732)
TimWolla added a commit that referenced this pull request Apr 12, 2026
* PHP-8.5:
  Fix order in NEWS
  Fix GH-21731: Random\Engine\Xoshiro256StarStar::__unserialize() accepts all-zero state (#21732)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants