Thanks for reporting this issue!

new SimpleXMLElement(…, …, true), DOMDocument::load(),
DOMDocument::loadHTMLFile(), XMLReader::open() and maybe others
are affected by this as well.

Yes other functions are also seems to be affected

any update are you guys working on the fix or not ? please let me know

Actually, the path is silently truncated in xmlParseURI() which
may be regarded as bug in libxml2.  We could catch that early,
though, and bail out:

 ext/libxml/libxml.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c
index c871cb89bd..6e450377f5 100644
--- a/ext/libxml/libxml.c
+++ b/ext/libxml/libxml.c
@@ -308,6 +308,9 @@ static void *php_libxml_streams_IO_open_wrapper(const char *filename, const char
 	int isescaped=0;
 	xmlURI *uri;
 
+	if (strstr(filename, "%00")) {
+		return NULL;
+	}
 
 	uri = xmlParseURI(filename);
 	if (uri && (uri->scheme == NULL ||


Stas, what do you think?

I am not entirely convinced it's either PHP problem or security problem, but I guess we could fix that. However, xmlParseURI() decodes URL-encoding, wouldn't that create other issues if you pass unsecured strings to it?

Also there's php_libxml_input_buffer_create_filename and php_libxml_output_buffer_create_filename - does it need to be changed too?

I think this is exactly the same issue like non percent-encoded
NUL bytes in paths, and so should be treated the same way.  The
fact that these functions generally accept percent-encoded
characters needs to be documented, but I don't see that we can
change that (in stable release branches), because existing code
may deliberately rely on that.

Anyhow, a full-fledged patch (guarding all xmlParseURI() uses):
<https://gist.github.com/cmb69/da6a17dc8604a331f3aa7f2860abaa49>.

If this patch has been implemented then I would like to request CVE for this please. It would be helpful for my resume/CV

Thank you so much 

<https://gist.github.com/cmb69/da6a17dc8604a331f3aa7f2860abaa49>

I'm still not sure how decoding %00 is different in principle from decoding %2F, for example. Both can be used to circumvent some user-space security mechanisms, if we worry about the former, shouldn't we also worry about the latter?

Basically %00 is NULL character and whenever we use NULL character then only the string before %00 will be considered as input.

I've also tried several URL encoding but only %00 was breaking the path of xml file.

Well, we should also worry about the undocumented percent-encoding
decoding in general (e.g. URI encoded directory traversal), but I
don't think we can do anything against it, but document the issue.

It just looks a bit weird. Maybe we should fix the percent-decoding part instead? I don't think anybody really relies on XML functions percent-decoding filenames. Percent in general is very rare in filenames but I think it's a bug if XML functions can't read any filenames that have %XX in them. I'd rather make it not percent-decode filenames instead.

If their is any other problem due to percentage encoding then You can raise an exception whenever someone uses percentage encoding in XML functions.

What do you think guys?

so what's the final decision? will you implement the fix of what please let me know. 

Thank you

Hi team,

If you have any update then please let me know. 

Thank you

<?php

$EXT = ".xml";
$FILE = "file"; //This may be controlled by user.

$FinalPath = $FILE+EXT /*File name can be controlled by user but extension is defined in php so their is no way to change extension*/

$path = "/home/aman/".$FILE;
echo simplexml_load_file($path);

?>

save this file as test.php and run this script. 

>> php test.php

now you will get the file.xml content as expected. 

lets see why I marked it as security issue. 

change the $FILE variable in above script from 'file' to '../../etc/passwd%00file'

now lets understand what's going on here so basically we are using directory transversal + null character to successfully reading the files on the server. 

I think it's a valid issue and it should be fixed and also assign a CVE. 

Thank you

No update?

Hi what about fix?

Hi team, Its my humble request to fix this issue If You find this a valid issue so that I can claim bounty for this issue from hackerone : https://hackerone.com/ibb-php.

I hope you'll understand

Thank you

Whenever we open a file, we feed the the result of xmlParseURI()
into xmlURIUnescapeString() and use this.  This way, the URI is
already percent decoded.  The only issue are %00, because these
are converted to NUL bytes, and since libxml2 deals with zero
terminated strings, that leads to truncation, so we need to catch
this (and only this).

DOMImplementation::createDocumentType() does not percent decode,
but I see no issue there, because if that URI is ever used to
retrieve the DTD, is would go through are loaders, which do the
percent decoding anyway.  Still, I think that catching %00 early
here is a good thing.

Thus, I propose to merge
<https://gist.github.com/cmb69/da6a17dc8604a331f3aa7f2860abaa49>.

If this issue will be fixed then. Will you assign CVE for this?

Yeah, that what is stopping me - why we are decoding it at all? simplexml_load_file() isn't supposed to url-decode filenames. I feel like we're paining over the real bug here.

I mean painting over.

> simplexml_load_file() isn't supposed to url-decode filenames.

So you are suggesting to not call xmlParseURI() in the first
place, but use the URL as is?  That might be the proper solution,
but I wouldn't want to make this change for any of the stable
branches (at least not PHP 7.4, let alone 7.3).  Maybe just apply
and release the patch, and do a PR for the proper behavior
afterwards?  This could then be discussed publicly.

I guess there's not much harm in applying it, even though it's not the proper solution.