Skip to content

stack overflow in json_encode() #15168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
YuanchengJiang opened this issue Jul 30, 2024 · 4 comments
Closed

stack overflow in json_encode() #15168

YuanchengJiang opened this issue Jul 30, 2024 · 4 comments

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
  
class Node
{
    /** @var Node */
    public $previous;
    /** @var Node */
    public $next;
}
$firstNode = new Node();
$firstNode->previous = $firstNode;
$firstNode->next = $firstNode;
$circularDoublyLinkedList = $firstNode;
for ($i = 0; $i < 200000; $i++) {
    $currentNode = $circularDoublyLinkedList;
    $nextNode = $circularDoublyLinkedList->next;
    $newNode = new Node();
    $newNode->previous = $currentNode;
    $currentNode->next = $newNode;
    $newNode->next = $nextNode;
    $nextNode->previous = $newNode;
    $circularDoublyLinkedList = $nextNode;
}
$random_var=$GLOBALS[array_rand($GLOBALS)];
json_encode($circularDoublyLinkedList);
?>

Resulted in this output:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==785404==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe577a9eb8 (pc 0x7f8c68704379 bp 0x7ffe577aa750 sp 0x7ffe577a9ec0 T0)
    #0 0x7f8c68704378 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:790
    #1 0x564774053b9d in smart_str_appendl_ex /php-src/Zend/zend_smart_str.h:130
    #2 0x564774053f42 in smart_str_appendl /php-src/Zend/zend_smart_str.h:168
    #3 0x56477405967f in php_json_escape_string /php-src/ext/json/json_encoder.c:365
    #4 0x564774055dfc in php_json_encode_array /php-src/ext/json/json_encoder.c:167
    #5 0x56477405d61d in php_json_encode_zval /php-src/ext/json/json_encoder.c:656
    #6 0x564774056115 in php_json_encode_array /php-src/ext/json/json_encoder.c:178
    #7 0x56477405d61d in php_json_encode_zval /php-src/ext/json/json_encoder.c:656
    #8 0x564774056115 in php_json_encode_array /php-src/ext/json/json_encoder.c:178
   ...
    #247 0x56477405d61d in php_json_encode_zval /php-src/ext/json/json_encoder.c:656
    #248 0x564774056115 in php_json_encode_array /php-src/ext/json/json_encoder.c:178

SUMMARY: AddressSanitizer: stack-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:790 in __interceptor_memcpy
==785404==ABORTING

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

@devnexen
Copy link
Member

can be reproduced with php 8.2

@arnaud-lb
Copy link
Member

Related / similar: #15169

@OlivierKessler01
Copy link

OlivierKessler01 commented Aug 26, 2024

@arnaud-lb

I don't know how that can help you, but I reproduce both bugs in PHP v7

json_encode() and serialize() don't work the same on cyclic directed graphs, json_encode() simply returns False, while serialize() handles recursion. I guess userspace would not be broken by making json_encode() return False before even completing a cycle.

Here is illustrating my point :

<?php

class Node
{
    /** @var Node */
    public $previous;
    /** @var Node */
    public $next;
}
$firstNode = new Node();
$firstNode->previous = $firstNode;
$firstNode->next = $firstNode;

$circularDoublyLinkedList = $firstNode;
$ser = serialize($circularDoublyLinkedList);
$jser = json_encode($circularDoublyLinkedList);
var_dump($jser); //bool(false) 
var_dump($ser); //string(49) "O:4:"Node":2:{s:8:"previous";r:1;s:4:"next";r:1;}"
?>

nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 25, 2024
The JSON encoder is recursive, and it's far from easy to make it
iterative. Add a cheap stack limit check to prevent a segfault.
This uses the PHP_JSON_ERROR_DEPTH error code that already talks about
the stack depth. Previously this was only used for the $depth argument.
@nielsdos nielsdos linked a pull request Sep 25, 2024 that will close this issue
nielsdos added a commit that referenced this issue Sep 30, 2024
* PHP-8.3:
  Fix GH-15168: stack overflow in json_encode()
nielsdos added a commit that referenced this issue Sep 30, 2024
* PHP-8.4:
  Fix GH-15168: stack overflow in json_encode()
jorgsowa pushed a commit to jorgsowa/php-src that referenced this issue Oct 1, 2024
The JSON encoder is recursive, and it's far from easy to make it
iterative. Add a cheap stack limit check to prevent a segfault.
This uses the PHP_JSON_ERROR_DEPTH error code that already talks about
the stack depth. Previously this was only used for the $depth argument.

Closes phpGH-16059.
jorgsowa pushed a commit to jorgsowa/php-src that referenced this issue Oct 1, 2024
The JSON encoder is recursive, and it's far from easy to make it
iterative. Add a cheap stack limit check to prevent a segfault.
This uses the PHP_JSON_ERROR_DEPTH error code that already talks about
the stack depth. Previously this was only used for the $depth argument.

Closes phpGH-16059.
@andypost
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants