GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Make igbinary_serialize()/igbinary_unserialize() match serialize()/unserialize() again by nicolas-grekas · Pull Request #423 · igbinary/igbinary

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented Sep 29, 2026

Copy link
Copy Markdown

serialize() and unserialize() changed quite a bit since PHP 7.4 and igbinary didn't follow, so that it's not a drop-in replacement anymore. This PR ports those changes, one commit per topic, best reviewed commit by commit:

  1. __sleep() skips uninitialized typed properties instead of throwing, skips missing properties, reports duplicate names, and serializes the object as null when it doesn't return an array, with the warnings of the running PHP version
  2. objects serialized as null (also when Serializable::serialize() returns null) don't shift the ids of later back-references anymore: such payloads couldn't be unserialized
  3. objects without a properties table are serialized from their slots, like serialize() does since 8.1, instead of allocating a table that stays attached to every object
  4. igbinary_unserialize() accepts the allowed_classes and max_depth options, and counts the depth on arrays and objects only
  5. class names, autoloader exceptions and properties sent to Serializable-only classes are checked like unserialize() does
  6. __unserialize() can overwrite readonly defaults on PHP 8.6
  7. two use-after-free: when a hook grows the array being serialized, and when unserialization fails after objects with pending __wakeup()/__unserialize() calls

The format doesn't change; some payloads get smaller since uninitialized properties aren't padded with nulls anymore.

Fix #123, fix #272, fix #273, fix #377, fix #402

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

michael-grunder reacted with thumbs up emoji tillkruss reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction
nicolas-grekas added 9 commits September 29, 2026 15:37
@nicolas-grekas
Fix use-after-free and leak in igbinary_serialize_array()
7828814
Hold a reference to the array while walking it, like serialize() does
since php/php-src#22714: a hook that appends to the array through a PHP
reference reallocated it in place and the iterator then read freed memory.

Also release the properties table on error paths. It leaked when a nested
value failed to serialize, which made igbinary_serialize_deep_nesting.phpt
report leaks on debug builds.
@nicolas-grekas
Make __sleep() handling match serialize()
3cb1546
Port php_var_serialize_get_sleep_props(): uninitialized typed properties
are skipped instead of throwing (igbinary#273, igbinary#377), missing properties are
skipped with a warning on PHP 8+, duplicate names are reported, and a
__sleep() that doesn't return an array serializes the object as null.
Warning levels and messages follow the running PHP version.

Objects serialized as null, by __sleep() or by Serializable::serialize()
returning null, are now removed from the table of reference ids:
igbinary_unserialize() doesn't count nulls, so later back-references
pointed to the wrong value or failed with "invalid reference".
@nicolas-grekas
Serialize declared properties without building the properties table
7ded337
Port the optimization serialize() got in PHP 8.1: objects that have no
properties table are serialized from their property slots. Building the
table left it allocated for the lifetime of each object, which made
memory usage grow after igbinary_serialize() (igbinary#402).

Uninitialized typed properties are now skipped instead of being written
as null padding, so the property count matches serialize() (igbinary#272).
@nicolas-grekas
Count unserialize depth on arrays and objects like unserialize()
f3b329b
unserialize_max_depth was checked for every value, including scalars, so
igbinary_unserialize() failed one level earlier than unserialize(). The
depth is now increased when entering a non-empty array or an object.

References to references, which igbinary_serialize() never emits, are
rejected so that recursion stays bounded by the depth limit.
@nicolas-grekas
Check class names and formats like unserialize() does
6d095ae
Reject empty class names, names with a leading NUL byte or namespace
separator, and names that are not valid class names, instead of creating
an incomplete class or resolving another class. Stop when an autoloader
throws, and lock the serializer state around autoloading and
unserialize_callback_func like unserialize() does.

Also fail with "Erroneous data format" when properties are provided for a
class that implements Serializable without __unserialize(): such classes
can only be created by their own unserialize() method.
@nicolas-grekas
Add the allowed_classes and max_depth options to igbinary_unserialize()
ee2e016
igbinary_unserialize() now accepts the same $options as unserialize()
(igbinary#123). Classes that are not allowed are unserialized as
__PHP_Incomplete_Class objects and enums are not affected, like with
unserialize(). The C API is unchanged.
@nicolas-grekas
Let __unserialize() overwrite the defaults of readonly properties
2090188
PHP 8.6 allows readonly properties to have a default value, and
unserialize() marks them as reinitializable while calling __unserialize()
(php/php-src#22588). Do the same, otherwise __unserialize() throws
"Cannot modify readonly property".
@nicolas-grekas
Don't expose partially unserialized objects on failure
1dfa7e1
Some failure paths released the partially unserialized value right away.
That ran destructors in the middle of unserialization and freed objects
still listed for a deferred __wakeup() or __unserialize() call, which
igbinary_unserialize_data_deinit() then wrote to. A truncated
__unserialize() payload was also deferred while uninitialized.

The partial value is now released once igbinary_unserialize() is done,
after objects that won't be woken up are marked as destructed, like
unserialize() does for the object that failed. Typed properties whose
value failed to unserialize are reset to their default, like
php/php-src#22481 does, and the serializer state is locked around
__wakeup().
@nicolas-grekas
Add missing tests to package.xml
d0fdc7a
@nicolas-grekas
nicolas-grekas force-pushed the serializer-parity branch from 2caeeda to d0fdc7a Compare October 14, 2026 09:44
@michael-grunder

michael-grunder commented Oct 28, 2026

Copy link
Copy Markdown

I ran into this while fuzz testing PhpRedis.

I created a small standalone reproducer:

<?php

class C {
    public function __serialize(): array { return ['a' => 1, 'b' => 2]; }
    public function __unserialize(array $data): void {}
}

$full = igbinary_serialize(new C);
$bad  = substr($full, 0, strpos($full, "\x14\x02") + 2); // header + class name + array8(len=2), zero elements

igbinary_unserialize($bad);
❯ USE_ZEND_ALLOC=0 valgrind php reproducer.php
==2382847== Memcheck, a memory error detector
==2382847== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==2382847== Using Valgrind-3.26.0 and LibVEX; rerun with -h for copyright info
==2382847== Command: php invalid.php
==2382847==
PHP Warning:  igbinary_unserialize_array: data size 0 smaller that requested array length 2. in /home/mike/dev/github/igbinary/invalid.php on line 29
==2382847== Conditional jump or move depends on uninitialised value(s)
==2382847==    at 0x45E0EA4: zval_ptr_dtor (in /usr/bin/php8.5)
==2382847==    by 0x7F5A93C: igbinary_unserialize_data_deinit (igbinary.c:2154)
==2382847==    by 0x7F5A93C: igbinary_unserialize (igbinary.c:813)
==2382847==    by 0x7F5AA83: zif_igbinary_unserialize (igbinary.c:834)
==2382847==    by 0x4584841: execute_ex (in /usr/bin/php8.5)
==2382847==    by 0x457E724: zend_execute (in /usr/bin/php8.5)
==2382847==    by 0x45E7C8F: zend_execute_script (in /usr/bin/php8.5)
==2382847==    by 0x447B1E4: php_execute_script_ex (in /usr/bin/php8.5)
==2382847==    by 0x45E9D74: ??? (in /usr/bin/php8.5)
==2382847==    by 0x418F118: ??? (in /usr/bin/php8.5)
==2382847==    by 0x5DC5F74: (below main) (libc_start_call_main.h:58)
==2382847==
survived
==2382847==
==2382847== HEAP SUMMARY:
==2382847==     in use at exit: 120,355 bytes in 1,733 blocks
==2382847==   total heap usage: 50,600 allocs, 48,867 frees, 8,603,384 bytes allocated
==2382847==
==2382847== LEAK SUMMARY:
==2382847==    definitely lost: 3,096 bytes in 20 blocks
==2382847==    indirectly lost: 8,486 bytes in 244 blocks
==2382847==      possibly lost: 0 bytes in 0 blocks
==2382847==    still reachable: 108,693 bytes in 1,468 blocks
==2382847==         suppressed: 80 bytes in 1 blocks
==2382847== Rerun with --leak-check=full to see details of leaked memory
==2382847==
==2382847== Use --track-origins=yes to see where uninitialised values come from
==2382847== For lists of detected and suppressed errors, rerun with: -s
==2382847== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nicolas-grekas

nicolas-grekas commented Oct 28, 2026

Copy link
Copy Markdown
Author

Thanks for the reproducer! That's the uninitialized __unserialize() payload fixed by 1dfa7e1: on this branch your script returns null with the warning and valgrind reports nothing, while on master it reads the uninitialized zval in igbinary_unserialize_data_deinit() (and debug builds abort on the assertion in igsd_defer_unserialize()). It's covered by tests/unserialize_failure_cleanup.phpt.

michael-grunder reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@michael-grunder

michael-grunder commented Oct 30, 2026

Copy link
Copy Markdown

I found a new leak while doing more PhpRedis fuzzing (patch).

It could either be added to this PR or I can open a new one.

Also any blockers to merging this PR? It's been running great for me locally.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nicolas-grekas nicolas-grekas mentioned this pull request Oct 30, 2026
[Cache][HttpFoundation] Deprecate igbinary and add phpser with HMAC support symfony/symfony#67668
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Strange memory behaviour when serializing objects Trigger error when trying to serialize Object with "__sleep" Skip uninitialized/unset typed properties returned from __sleep() in php 8.0+ Skip uninitialized/unset typed properties when serializing objects in php 7.4+ Support allowed_classes option, like unserialize?

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@nicolas-grekas @michael-grunder
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.