While I understand that exploitation of this issue depends heavily on bad coding practices by the user, but as libcurl stops processing the path as soon as it finds a NULL byte (see https://github.com/bagder/curl/blob/master/lib/file.c, line 228), there is no need for these bytes to be accepted in path in the first place.

I think this is a curl issue. %00 is valid in URLs. We might add a mitigation for the file:// protocol, not sure about that.

There however is a related issue we certainly have to fix: curl_easy_setopt() is not binary safe, using
   $url = "file:///etc/passwd\0http://google.com";
shows that issue. This one is fixed in the attached patch.

One should actually use CURLOPT_PROTOCOLS to limit the protocol.

Arbitrary restricting valid URI (as Johannes) may be wrong.

We could enforce the given protocol only for anything remote, that would solve the local data access using "crafted" URI.

Thanks for getting back to me. Will you be requesting a CVE-ID, or would you like me to do a write up and request a CVE-ID?

I discussed this issue with Bagder and for file: it could be done in curl. Patch:

https://gist.github.com/bagder/3b41a03b5feee6a59b0f

That will at least solve when one uses the next release. Or if distributions backport it.

Not sure we need a CVE for that.

@research at g0blin dot co dot uk

for the curl credit page/file, do you prefer your email or real name?

Do we really need a CVE here? Bagder or I thought it is not, would not mind tho' :) he is on Freenode's #curl 

Also we should follow up with curl as the fix will happen in curl and it is actually a curl issue.

We should also improve the documentation for the CURLOPT_PROTOCOLS usage to fix it  for current code.

My email address will suffice, thanks.

With regards to a CVE-ID, I can see several other bugs like this that have CVE-IDs against them. As this could potentially be used to disclose arbitrary files by user input, do you not think it should have a CVE-ID?

Correct me if I'm wrong - I only started doing this kind of research recently after many years in development, but I have an example around somewhere that allows this exact issue to occur, due to a particular PHP script, which takes user input as a partial in a path, which is passed to curl_setopt. Again, bad programming practice, but it shouldn't of been possible IMO.

Happy to go with what you decide RE this. I'll likely do a small write-up in the near future, regardless of CVE assignment or not, as I found it a useful learning experience.

James H

Point taken - this really is an issue with cURL, not PHP, so we do not need a CVE against PHP.

Before this is overseen: There *is* a PHP issue, though it is not directly the reported one. Just put a \0 the in any option (like the URL instead of %00). This has to be fixed on our side. See attached patch (which is missing a test right now)

I don't seem to be able to view the patch - which I'd like to, simply out of curiosity. Should I wait until this bug is set as public, or could you provide me with access to the patch diff?

trivial patch:

diff --git a/ext/curl/interface.c b/ext/curl/interface.c
index d7cacf5..094c8ae 100644
--- a/ext/curl/interface.c
+++ b/ext/curl/interface.c
@@ -170,6 +170,11 @@ static int php_curl_option_str(php_curl *ch, zend_long option, const char *str,
 {
 	CURLcode error = CURLE_OK;
 
+	if (strlen(str) != len) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Curl option %d contains invalid characters (\\0) ignoring");;
+		return FAILURE;
+	}
+
 #if LIBCURL_VERSION_NUM >= 0x071100
 	if (make_copy) {
 #endif

Trivial and wrong - format string with %d but no parameter ... but shows the point

The patch applies for 5.5+, however I wonder if we need a fix for 5.4 too, at least for url ones?

FYI, bagder released an update to the curl repo that prevents null byte escaping in file paths. I understand this does not fix the null byte escaping in php_curl_option_str, but just thought I'd put the change on this bug report.

https://github.com/bagder/curl/commit/53cbea22310f1509e98f5537ef3a83c6e600629f

Cheers