stack overflow when var serialization in ext/standard/var

[YuanchengJiang]

Description

The following code:

<?php
function var_fusion($var1, $var2, $var3) {
$vars = [$var1, $var2, $var3];
foreach ($vars as $i => $v1) {
foreach ($vars as $j => $v2) {
if ($i < $j) {
try {
$result["serialize_{$j}"] = serialize($v2);
} catch (Exception $e) {
}
}
}
}
}
class Node
{
public $next;
}
$firstNode = new Node();
$circularDoublyLinkedList = $firstNode;
for ($i = 0; $i < 200000; $i++) {
$currentNode = $circularDoublyLinkedList;
$nextNode = $circularDoublyLinkedList->next;
$newNode = new Node();
$currentNode->next = $newNode;
$newNode->next = $nextNode;
}
$script2_connect=$circularDoublyLinkedList;
var_fusion($script1_connect, $script2_connect, $random_var);

Resulted in this output:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==1985328==ERROR: AddressSanitizer: stack-overflow on address 0x7fff2a15be80 (pc 0x55974e0c4609 bp 0x7fff2a15cf10 sp 0x7fff2a15be80 T0)
    #0 0x55974e0c4609 in php_var_serialize_intern /WorkSpace/php-src/ext/standard/var.c:983
    #1 0x55974e0ca7ee in php_var_serialize_intern /WorkSpace/php-src/ext/standard/var.c:1249:8
    ...
    #246 0x55974e0ca7ee in php_var_serialize_intern /WorkSpace/php-src/ext/standard/var.c:1249:8

SUMMARY: AddressSanitizer: stack-overflow /WorkSpace/php-src/ext/standard/var.c:983 in php_var_serialize_intern

Valgrind:

==797883== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==797883== 
==797883== Process terminating with default action of signal 11 (SIGSEGV)
==797883==  Access not within mapped region at address 0x1FFE801D28
==797883== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==797883==    at 0x710C2E: php_var_serialize_intern (var.c:983)
==797883==  If you believe this happened as a result of a stack
==797883==  overflow in your program's main thread (unlikely but
==797883==  possible), you can try to increase the size of the
==797883==  main thread stack using the --main-stacksize= flag.
==797883==  The main thread stack size used in this run was 8388608.
==797883== Stack overflow in thread #1: can't grow stack to 0x1ffe801000

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

[devnexen]

can be reproduced with php 8.2

[arnaud-lb]

Simpler reproducer:

<?php

class Node
{
    public $next;
}

$firstNode = new Node();
$node = $firstNode;
for ($i = 0; $i < 200000; $i++) {
    $newNode = new Node();
    $node->next = $newNode;
    $node = $newNode;
}

serialize($firstNode);

We could add a manual stack limit check in php_var_serialize_intern() to avoid that.

Unfortunately we will also enter deep recursion when destroying $firstNode recursively.

[bukka]

Yeah it's crashing even without serialize on object dtor recursive destroying. So we have it in serialize, obj dtor and json_encode and I wouldn't be suprised if there are more places causing the crash.

Those zend_call_stack_overflowed called from seems pretty lightweight to me so we could just add zend_check_stack_limit (or its variant) to the right places if we want some custom messages for json or serialize (probably not worth it). Do you see any potential issue with that @arnaud-lb ?

[arnaud-lb]

I don't see issues with that 👍 It may be worth it to run a benchmark just to be sure that there is no performance regression.

Alternatively we could switch to a non-recursive algorithm, if this does not increase complexity too much.

[andypost]

somehow Alpinelinux s390x builder been stuck after this change so filed #16528