Edit report at https://bugs.php.net/bug.php?id=62577&edit=1
ID: 62577
Updated by: cmb@php.net
Reported by: ivan dot enderlin at hoa-project dot net
Summary: simplexml_load_file does not file if
libxml_disable_entity_loader(true)
Status: Open
Type: Bug
Package: SimpleXML related
Operating System: All
PHP Version: master-Git-2012-07-16 (Git)
Block user comment: N
Private report: N
New Comment:
Hmm, I wonder why we check whether external entity loading is
disabled in php_libxml_input_buffer_create_filename()[1] (which is
the xmlParserInputBufferCreateFilenameDefault() callback), instead
of in _php_libxml_external_entity_loader()[2] (which is the
xmlSetExternalEntityLoader() callback). (See the attached
move-entity_loader_disabled-check patch.) Wouldn't the latter be
sufficient to prevent XXE attacks?
Also I wonder whether we need libxml_disable_entity_loader() at
all. Only if LIBXML_DTDLOAD|LIBXML_NOENT are given as options,
external entities will be resolved. Some of the XML parsers don't
accept options, but at least as of libxml 2.9.0 save defaults are
used anyway[3].
[1] <https://github.com/php/php-src/blob/php-7.3.0RC3/ext/libxml/libxml.c#L395>
[2] <https://github.com/php/php-src/blob/php-7.3.0RC3/ext/libxml/libxml.c#L572>
[3] <https://gitlab.gnome.org/GNOME/libxml2/commit/4629ee02ac649c27f9c0cf98ba017c6b5526070f>
Previous Comments:
------------------------------------------------------------------------
[2018-10-15 10:50:01] cmb@php.net
The following patch has been added/updated:
Patch Name: move-entity_loader_disabled-check
Revision: 1539600601
URL: https://bugs.php.net/patch-display.php?bug=62577&patch=move-entity_loader_disabled-check&revision=1539600601
------------------------------------------------------------------------
[2018-05-22 11:12:00] phofstetter at sensational dot ch
> and if you don't give valid path, you get an error and false.
of course. But this bug is about simplexml_load_file failing on *any* valid path if
libxml_disable_entity_loader(true) has been called.
Here's a test script. IMHO, both assert()s should pass:
<?php
file_put_contents('/tmp/test.xml',
'<doc><foo>bar</foo></doc>');
libxml_disable_entity_loader(false);
assert(simplexml_load_file('/tmp/test.xml')->foo == 'bar');
libxml_disable_entity_loader(true);
assert(simplexml_load_file('/tmp/test.xml')->foo == 'bar');
unlink('/tmp/test.xml');
------------------------------------------------------------------------
[2018-05-22 09:34:19] cojubacaso at stelliteop dot info
I don't see how this is a bug, the function is called "simplexml_load_file", so the
expected behavior is that it will load content of a file, and if you don't give valid path, you
get an error and false.
It is also documented like that, so please just close this, changing this behavior will probably
brake a lot of applications also.
------------------------------------------------------------------------
[2016-10-17 13:32:58] cmb@php.net
Related To: Bug #73328
------------------------------------------------------------------------
[2016-10-03 20:22:58] gudang at gmail dot com
@rrichards When are you going to fix this 4 years issue?
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=62577
--
Edit this bug report at https://bugs.php.net/bug.php?id=62577&edit=1