Skip to content

include path resolution of relative paths inconsistent - potential security issue #21488

@kkmuffme

Description

@kkmuffme

Description

Initially, I wanted to just file this in php-doc as an issue, until I realized there's actually multiples bugs - besides lots of unexpected behavior.

The following code:

/some/includes/a.php

<?php
file_put_contents( __DIR__ . '/b.php', '<?php echo "b" . PHP_EOL;' );
mkdir( __DIR__ . '/nested/nested', 0755, true );
file_put_contents( __DIR__ . '/nested/nested/x.php', '<?php include "b.php"; echo "x" . PHP_EOL;' );
file_put_contents( __DIR__ . '/nested/nested/b.php', '<?php echo "BX" . PHP_EOL;' );
mkdir( __DIR__ . '/c' );
chdir( __DIR__ . '/c' );
file_put_contents( __DIR__ . '/c/b.php', '<?php echo "C" . PHP_EOL;' );

var_dump( get_include_path() );
include __DIR__ . '/nested/nested/x.php';
var_dump( file_get_contents( 'b.php' ) );
var_dump( file_get_contents( './b.php' ) );
var_dump( file_exists( 'b.php' ) );
var_dump( file_exists( './b.php' ) );
require 'b.php';
require './b.php';

Resulted in this output:

  1. set php.ini include_path = "/dev/null"; (or put set_include_path( '/dev/null' ); on top of a.php)

php includes/a.php

string(9) "/dev/null"
BX
x
string(25) "<?php echo "C" . PHP_EOL;"
string(25) "<?php echo "C" . PHP_EOL;"
bool(true)
bool(true)
b
C


  1. change php.ini include_path = ".:/dev/null;"

php includes/a.php

string(11) ".:/dev/null"
C
x
string(25) "<?php echo "C" . PHP_EOL;"
string(25) "<?php echo "C" . PHP_EOL;"
bool(true)
bool(true)
C
C


  1. now remove the file_put_contents( __DIR__ . '/c/b.php', '<?php echo "C" . PHP_EOL;' ); line
    set php.ini include_path = /dev/null;

php includes/a.php

string(9) "/dev/null"
BX
x
E_WARNING: file_get_contents(b.php): Failed to open stream: No such file or directory
bool(false)
E_WARNING: file_get_contents(./b.php): Failed to open stream: No such file or directory
bool(false)
bool(false)
bool(false)
b
E_WARNING: require(./b.php): Failed to open stream: No such file or directory
Error: Failed opening required './b.php' (include_path='/dev/null')

  1. like in 3) remove the file_put_contents( __DIR__ . '/c/b.php', '<?php echo "C" . PHP_EOL;' ); line
    change php.ini include_path = ".:/dev/null;"

php includes/a.php

string(11) ".:/dev/null"
BX
x
E_WARNING: file_get_contents(b.php): Failed to open stream: No such file or directory
bool(false)
E_WARNING: file_get_contents(./b.php): Failed to open stream: No such file or directory
bool(false)
bool(false)
bool(false)
b
E_WARNING: require(./b.php): Failed to open stream: No such file or directory
Error: Failed opening required './b.php' (include_path='.:/dev/null')

But I expected this output instead:
It should behave in a simple and predictable way that is consistent in all cases.
This means that:

  • ./ and ../ behave like __DIR__ . '/' and dirname( __DIR__ ) . '/' only
  • . in include_path setting in .ini behaves like __DIR__ . '/'
  • relative paths like b.php if fail to resolve using the include_path, are attempted to be resolved like __DIR__ . '/b.php' only
  • file_exists/... and include/... should use the same relative path resolution logic in all cases (however, if the above 3 points are fixed, this will be fixed automatically)

Essentially: never use the current working directory (or even worse: the calling script's directory) to resolve relative paths, always resolve it relative to the current file.

Problems still: code that runs set_include_path() - which is often done temporarily and later reverted (e.g. phpunit does/used to do that) - will permanently alter the include path, if it throws
And will also unexpectedly modify the include path for unrelated callbacks (ob_start callback, autoloaders) that execute but were specified outside of the current code (which allows for easy supply chain attacks)

Issues:

  • Makes no sense that 1 is different to 2, but 3 identical to 4
  • file_exists/is_file/... as well as file_get_contents/file_put_contents/... behave differently than require/include in some cases
  • due to side-effects which can call chdir() or set_include_path()/ini_set(), it's impossible for anybody to know for any code how a relative path will get resolved and what file will actually be loaded (which can lead to security vulnerabilities), e.g.
    -- class_exists(), new ..., calling a static method Foo::run() -> calls autoloader
    -- any code that results in a PHP notice/warning/exception -> calls error/exception handler
    -- flush() - if headers haven't been sent, calls header_register_callback()
    -- ob_flush(), ob_get_clean(),... -> calls the ob_start callback
    -- echo, printf,... -> if ob_implicit_flush is on and ob_start is declared with a size of 1, it will call the ob_start callback for every echo/printf/...
    -- exit -> calls register_shutdown_function
    probably various additional cases I forgot about
  • https://www.php.net/manual/en/function.include.php

include will finally check in the calling script's own directory and the current working directory before failing

is true for "calling script's own directory" only in some cases as shown above.
Also it's ambiguous if "calling script" means the current PHP file executing or the initial PHP file (SCRIPT_FILENAME)

If a path is defined — whether absolute (starting with a drive letter or \ on Windows, or / on Unix/Linux systems) or relative to the current directory (starting with . or ..) — the include_path will be ignored altogether. For example, if a filename begins with ../, the parser will look in the parent directory to find the requested file.

Is wrong, since it's the current **working** directory in some cases and parent directory **of the current working directory**

And the "include_path" is not ignored if it's a relative path - while the docs are technically correct in this regard, since they do not state that, it's unexpected for most people when they read "relative path"

It's probably even more confusing when using symlinks, so I didn't check that, but I'm sure there's probably more issues there.

Fixing this to be consistent and behave as expected in all cases probably requires some major work.
I guess it would be easier and safer to:

  • Deprecate include_path in .ini and set_include_path()/get_include_path() functions
  • Deprecate relative file paths in all file handling functions
    Advantages:
  • __DIR__ is available since PHP 5.3.0 and always contains an absolute path, which makes the use of relative paths unnecessary to begin with
  • consistent, expected behavior in all cases
  • safer, since no more unexpected loading of files
  • faster
  • allows for better static analysis
    Disadvantage:
  • long term: breaking change that requires RFC (short term: emit E_DEPRECATED for it)

PHP Version

All PHP versions including PHP 8.5, with slight differences in behavior in some versions <PHP 8.2

Operating System

No response

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions