php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64938 libxml_disable_entity_loader setting is shared between requests (FPM)
Submitted: 2013-05-28 13:43 UTC Modified: 2017-11-27 16:01 UTC
Votes:58
Avg. Score:4.5 ± 0.9
Reproduced:47 of 48 (97.9%)
Same Version:13 (27.7%)
Same OS:9 (19.1%)
From: Sjon at hortensius dot net Assigned: remi (profile)
Status: Closed Package: *XML functions
PHP Version: 5.4.15 OS: Archlinux
Private report: No CVE-ID: 2015-8866
 [2013-05-28 13:43 UTC] Sjon at hortensius dot net
Description:
------------
The libxml_disable_entity_loader() setting is shared between hits in a FPM 
process. Therefore; our script have the external entity-loader randomly 
enabled/disabled.

Test script:
---------------
<?php

die(var_dump(libxml_disable_entity_loader(false)));

Expected result:
----------------
The default setting (which is true since 5.4.13) should always be var_dump-ed

Actual result:
--------------
since this setting is remembered in the thread; after a while all hits return 
false

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-05-30 21:30 UTC] Sjon at hortensius dot net
-Summary: libxml_disable_entity_loader setting is shared between hits +Summary: libxml_disable_entity_loader setting is shared between threads
 [2013-05-30 21:30 UTC] Sjon at hortensius dot net
Clarified summary
 [2013-10-10 12:03 UTC] mike@php.net
-Package: FPM related +Package: *XML functions
 [2015-01-29 16:44 UTC] stefan dot behninger at nasdaq dot com
Seems like this is a much bigger issue. We discovered that disabling the loader does not only affect the current thread but obviously changes the setting globally on the entire server. Plus, it seems to be persisted in a way that only restarting the server got us back to normal.

Planning to do some more tests tomorrow to eliminate any kind of caching that might have been involved.

We're on PHP 5.3.3 on CentOS, strictly single-threaded.
 [2015-02-01 08:10 UTC] stas@php.net
Automatic comment on behalf of martin@divbyzero.net
Revision: http://git.php.net/?p=php-src.git;a=commit;h=de31324c221c1791b26350ba106cc26bad23ace9
Log: Fix bug #64938: libxml_disable_entity_loader setting is shared between threads
 [2015-02-01 08:10 UTC] stas@php.net
-Status: Open +Status: Closed
 [2015-02-01 08:10 UTC] stas@php.net
Automatic comment on behalf of martin@divbyzero.net
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c1eb87ab1a2e2df1868b70cd7b8016c6147092c5
Log: Fix bug #64938: libxml_disable_entity_loader setting is shared between threads
 [2015-04-29 11:57 UTC] freitsabes at gmail dot com
Sorry, but I would like to ask for clarification:
For php-cgi I see the following behaviour:
1. I issue a request that has a call of libxml_disable_entity_loader()
2. A subsequent request to a different script that is NOT calling libxml_disable_entity_loader is affected by the first request because the setting is shared between subsequent requests on the same process.

Is this a bug or working as intended?

PHP 5.4.39 with libxml 2.9.2
 [2015-10-16 12:46 UTC] mark at netalico dot com
Any suggested workarounds for this issue? This bug is pretty critical because it can basically take down sites running something like Magento. It appears to only be fixed in PHP 5.6, which a lot of codebases aren't ready for yet.
 [2015-11-25 08:52 UTC] kaplan@php.net
Also fixed in 5.5.22 (per the commits above).
 [2015-12-22 12:26 UTC] robert dot egginton at c3media dot co dot uk
I'm using 5.5.30 and php-fpm and can reproduce the problem by using the inverse of the script:

Test script:
---------------
<?php

die(var_dump(libxml_disable_entity_loader(true)));

---------------

The default seems to be false for me. After a few hits the results all end up true, so somehow this value is persisting within php-fpm children.
 [2015-12-22 12:35 UTC] robert dot egginton at c3media dot co dot uk
For mark at netalico dot com:

The workaround for something like Magento (not required for CE>1.9.2.0 when a workaround was added) is to add this line to the start of your script:

if (function_exists('libxml_disable_entity_loader')) {
    libxml_disable_entity_loader(false);
}
 [2016-04-26 09:14 UTC] remi@php.net
-Assigned To: +Assigned To: remi -CVE-ID: +CVE-ID: 2015-8866
 [2016-07-20 11:39 UTC] davey@php.net
Automatic comment on behalf of martin@divbyzero.net
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c1eb87ab1a2e2df1868b70cd7b8016c6147092c5
Log: Fix bug #64938: libxml_disable_entity_loader setting is shared between threads
 [2016-10-10 10:27 UTC] robert dot egginton at c3media dot co dot uk
I can confirm that I still get the issue with 7.0.11. I run my above checking script and after a few runs all entries come out as true, so this is still not thread safe.
 [2016-10-12 09:33 UTC] ntd at entidi dot it
The problem is still present in php-fpm 5.6.26 (debian 8.6).

Commit c1eb87ab1a2e2df1868b70cd7b8016c6147092c5 was pushed 20 months ago, so I suppose it did not fix the problem.
 [2016-10-14 13:38 UTC] cmb@php.net
-Status: Closed +Status: Re-Opened
 [2016-10-14 13:38 UTC] cmb@php.net
Re-opened according to recent user comments (thanks!)
 [2017-10-24 05:15 UTC] kalle@php.net
-Status: Re-Opened +Status: Assigned
 [2017-11-27 15:59 UTC] remi@php.net
Can someone test this simple fix proposal

diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c
index d88860c..bfc1224 100644
--- a/ext/libxml/libxml.c
+++ b/ext/libxml/libxml.c
@@ -848,7 +848,6 @@ static PHP_MINIT_FUNCTION(libxml)
        if (sapi_module.name) {
                static const char * const supported_sapis[] = {
                        "cgi-fcgi",
-                       "fpm-fcgi",
                        "litespeed",
                        NULL
                };
 [2017-11-27 16:01 UTC] remi@php.net
-Summary: libxml_disable_entity_loader setting is shared between threads +Summary: libxml_disable_entity_loader setting is shared between requests (FPM)
 [2017-11-28 17:00 UTC] remi@php.net
Automatic comment on behalf of remi@remirepo.net
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8e5b9532da0308c50c9cb316e9fda530becdc543
Log: Fixed bug #64938 libxml_disable_entity_loader setting is shared between requests (FPM)
 [2017-11-28 17:00 UTC] remi@php.net
-Status: Assigned +Status: Closed
 [2017-11-29 12:01 UTC] maunel-php at mausz dot at
@remi

Just noticed that there's a bug report for this. We're shipping the following patch since noticing the issue ourself:

diff -Naur php-7.2.0.orig/ext/libxml/libxml.c php-7.2.0/ext/libxml/libxml.c
--- php-7.2.0.orig/ext/libxml/libxml.c  2017-11-12 19:03:42.890478753 +0100
+++ php-7.2.0/ext/libxml/libxml.c       2017-11-12 19:03:58.510298201 +0100
@@ -880,13 +880,14 @@
                xmlSetGenericErrorFunc(NULL, php_libxml_error_handler);
                xmlParserInputBufferCreateFilenameDefault(php_libxml_input_buffer_create_filename);
                xmlOutputBufferCreateFilenameDefault(php_libxml_output_buffer_create_filename);
-
-               /* Enable the entity loader by default. This ensures that
-                * other threads/requests that might have disabled the loader
-                * do not affect the current request.
-                */
-               LIBXML(entity_loader_disabled) = 0;
        }
+
+       /* Enable the entity loader by default. This ensures that
+        * other threads/requests that might have disabled the loader
+        * do not affect the current request.
+        */
+       LIBXML(entity_loader_disabled) = 0;
+
        return SUCCESS;
 }

No idea which one is better. Just wanted to share this.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 05:01:29 2024 UTC