realpath and readfile (and probably others in zend_virtual_cwd) fail with valid path of length 4095

[SlivTaMere]

Description

The following code:

<?php
$p = "/bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/../bin/sh";
var_dump(PHP_MAXPATHLEN);
var_dump(strlen($p));
var_dump(file_exists($p));
var_dump(realpath($p));

Resulted in this output:

int(4096)
int(4095)
bool(true)
bool(false)

But I expected this output instead:

int(4096)
int(4095)
bool(true)
string(7) "/bin/sh" //or similar

I didn't dig much but I think the issue is at Zend/zend_virtual_cwd.c , a path of length 4095 should be accepted as it is the case with file_exists() and stat family functions.

I didn't test with PHP 8 but probably has the same issue.

PHP Version

PHP 7.4.33

Operating System

Ubuntu 18.04.6

[cmb69]

On non Windows systems, PHP defines its MAXPATHLEN according to POSIX's PATH_MAX or MAX_PATH if available:

php-src/Zend/zend_virtual_cwd.h

Lines 39 to 50 in b3f3414

	#ifndef MAXPATHLEN
	# if _WIN32
	# include "win32/ioutil.h"
	# define MAXPATHLEN PHP_WIN32_IOUTIL_MAXPATHLEN
	# elif PATH_MAX
	# define MAXPATHLEN PATH_MAX
	# elif defined(MAX_PATH)
	# define MAXPATHLEN MAX_PATH
	# else
	# define MAXPATHLEN 256
	# endif
	#endif

These are not unlikely to default to 4096 bytes (or even less), and I don't think there is anything we can do.

[SlivTaMere]

The issue I'm trying to convey is that realpath() and similar functions don't work with a valid path of 4095 (MAX_PATH-1) whereas functions like file_exists() do succeed.

This might lead to unexpected behavior where file_exists() and is_readable() return true but realpath() and readfile() will fail and return false.

[Tontyna]

@SlivTaMere is right, the comparison in

php-src/Zend/zend_virtual_cwd.c

Line 1025 in c40dcf9

if (!path_length || path_length >= MAXPATHLEN-1) {

should be

if (!path_length || path_length+1 >= MAXPATHLEN-1) {

We need 1 byte for the null-termination, not 2.

[cmb69]

if (!path_length || path_length+1 >= MAXPATHLEN-1) {

That would basically be equivalent to

if (!path_length || path_length >= MAXPATHLEN-2) {

so the path would even have to be shorter.

[Tontyna]

Upps, of course you are right. Alt+Backspace, undo, ignore...

It's actually

php-src/Zend/zend_virtual_cwd.c

Line 1074 in c40dcf9

if (path_length + state_cwd_length + 1 >= MAXPATHLEN-1) {

getting it wrong.

Nah, should read the complete code before telling things. Sorry for the noise.

[cmb69]

Nah, should read the complete code before telling things.

Hmm, not sure. It seems that +1 is for the trailing slash, but if it is already there, its length shouldn't be added for that check.

[iluuu1994]

Various checks fail, like path_length >= MAXPATHLEN-1. I'm not exactly sure why >= instead of just >. It also seems like virtual_file_ex() (which I don't understand because it's horribly complex) also seems to require a few bytes to move things around. I don't think making big changes to that function is worth the hassle.

[iluuu1994]

@nielsdos also just mentioned that there was a security issue in that code recently, which confirms the fact that we shouldn't touch that code. PHP requiring a few bytes from MAXPATHLEN for path resolution doesn't seem like a big deal. It's not great that there are inconsistencies across functions but it's not worth opening a can of worms.