Bug: a tilde (~) in the path, such as within iCloud local storage, will cause CodeIgniter to fail

[zigmoo]

PHP Version

8.2

CodeIgniter4 Version

4.2.11

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

cli-server (PHP built-in webserver)

Database

none

What happened?

When a fresh composer-created CodeIgniter 4 application has a tilde(~) in its path, it crashes when staring with "php spark serve".

This is a critical failure, because on MacOS in all filesystem areas synced to iCloud, tildes are present, like so: /Users/moo/Library/Mobile Documents/com~apple~CloudDocs/Documents/development/swapper/vendor/codeigniter4

I moved the fresh new app out of the com~apple~CloudDocs directory tree to /Users/shared/dev/swapper, and it immediately starts up fine with "php spark serve".

After renaming the directory from "/Users/shared/dev/swapper" to "/Users/shared/dev/~swapper", the application immediately fails when started with "php spark serve".

CodeIgniter_4.2.11_tilde_bug.pdf

Steps to Reproduce

1. cd to directory that does not include a tilde(~), such as /Users/shared/ on MacOS.
2. composer create-project codeigniter4/appstarter newApp
3. cd ./newApp
4. php spark serve
5. newApp works as expected
6. cd ../
7. mv ./newApp ./\~newApp
8. cd ./\~newApp
9. php spark serve
10. newApp fails due to the tilde-intolerance bug
11. cd ../
12. mv ./\~newApp ./newApp
13. cd newApp
14. php spark serve now works as expected

Expected Output

php spark serve should start the application, no matter what characters are present in the path name. And ESPECIALLY tildes should be accounted for, because all MacOS users should be using iCloud to sync their files.

Anything else?

The test log on my machine:

{23-01-05 23:51}MoosacreM1ProMax:/Users/Shared/dev moo% cd swapper 
{23-01-05 23:51}MoosacreM1ProMax:/Users/Shared/dev/swapper moo% php spark serve

CodeIgniter v4.2.11 Command Line Tool - Server Time: 2023-01-05 23:51:41 UTC-06:00

CodeIgniter development server started on http://localhost:8080
Press Control-C to stop.
[Thu Jan  5 23:51:41 2023] PHP 8.2.1 Development Server (http://localhost:8080) started
^C
{23-01-05 23:51}MoosacreM1ProMax:/Users/Shared/dev/swapper moo% cd ..
{23-01-05 23:51}MoosacreM1ProMax:/Users/Shared/dev moo% mv swapper \~swapper
{23-01-05 23:52}MoosacreM1ProMax:/Users/Shared/dev moo% cd \~swapper 
{23-01-05 23:52}MoosacreM1ProMax:/Users/Shared/dev/~swapper moo% php spark serve
PHP Fatal error:  Uncaught Error: Class "Config\App" not found in /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Factories.php:129
Stack trace:
#0 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Common.php(204): CodeIgniter\Config\Factories::__callStatic('config', Array)
#1 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Services.php(146): config('App')
#2 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/BaseService.php(253): CodeIgniter\Config\Services::codeigniter(NULL, false)
#3 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/BaseService.php(194): CodeIgniter\Config\BaseService::__callStatic('codeigniter', Array)
#4 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Services.php(143): CodeIgniter\Config\BaseService::getSharedInstance('codeigniter', NULL)
#5 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/BaseService.php(253): CodeIgniter\Config\Services::codeigniter()
#6 /Users/Shared/dev/~swapper/spark(82): CodeIgniter\Config\BaseService::__callStatic('codeigniter', Array)
#7 {main}
  thrown in /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Factories.php on line 129

Fatal error: Uncaught Error: Class "Config\App" not found in /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Factories.php:129
Stack trace:
#0 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Common.php(204): CodeIgniter\Config\Factories::__callStatic('config', Array)
#1 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Services.php(146): config('App')
#2 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/BaseService.php(253): CodeIgniter\Config\Services::codeigniter(NULL, false)
#3 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/BaseService.php(194): CodeIgniter\Config\BaseService::__callStatic('codeigniter', Array)
#4 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Services.php(143): CodeIgniter\Config\BaseService::getSharedInstance('codeigniter', NULL)
#5 /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/BaseService.php(253): CodeIgniter\Config\Services::codeigniter()
#6 /Users/Shared/dev/~swapper/spark(82): CodeIgniter\Config\BaseService::__callStatic('codeigniter', Array)
#7 {main}
  thrown in /Users/Shared/dev/~swapper/vendor/codeigniter4/framework/system/Config/Factories.php on line 129
{23-01-05 23:52}MoosacreM1ProMax:/Users/Shared/dev/~swapper moo% composer create-project codeigniter4/appstarter swapper

[kenjis]

Thank you for reporting. This is not a bug.

CodeIgniter autoloader does not allow special characters that are illegal in filenames on certain operating systems. The symbols that can be used are /, _, ., :, \ and space. So if you install CodeIgniter under the folder that contains the special characters like (, ), etc., CodeIgniter won’t work. https://codeigniter4.github.io/CodeIgniter4/installation/installing_composer.html#installation

php spark serve should start the application, no matter what characters are present in the path name.

Why do you think so? Are you sure this CodeIgniter sanitization process is absolutely unnecessary?

It came from: https://github.com/codeigniter4/CodeIgniter4/commit/62381f3c

[kenjis]

The current code: https://github.com/codeigniter4/CodeIgniter4/blob/d8d52757b38c16a836e5296c331ecf9a6e9f324d/system/Autoloader/Autoloader.php#L302-L312 https://github.com/codeigniter4/CodeIgniter4/blob/d8d52757b38c16a836e5296c331ecf9a6e9f324d/system/Autoloader/Autoloader.php#L278-L289

@codeigniter4/core-team At least, just using the sanitizeFilename() method when autoloading a file is not good. Because if a filename is sanitized, the autoloading will fail surely, but the error message is like:

Class "Config\App" not found

It does not tell why not found. It seems better throwing an exception with a proper message if a filename is sanitized.

Also, I'm not sure how much sense it makes to do this sanitization process during autoloading. But I also don't have a reason why it should be actively removed.

[MGatner]

The code already states:

Modified to allow backslash and colons for on Windows machines.

I think we should just expand the regex to include non-initial tildes.

[paulbalandan]

In the first place, why do we need to sanitize the filename prior to autoloading? What risks are we preventing? Path traversal attacks?

[kenjis]

I think we need to separate the cases of autoloading and other cases.

Suppose the autoloader tries to load a class name from user input, what are the possible risks?

[kenjis]

Possible vulnerability in autoloading is Local File Inclusion.

If an attacker is able to include a file with an arbitrary path on the server, s/he can execute arbitrary code on the server if s/he is able to upload a file to the server.

[kenjis]

It seems . is not permitted unless PHP 5.3.0 - 5.3.29, 5.4.0 - 5.4.23, 5.5.0 - 5.5.7 https://3v4l.org/SUUZe

[zigmoo]

The code already states:

Modified to allow backslash and colons for on Windows machines.

I think we should just expand the regex to include non-initial tildes.

Agreed!

[zigmoo]

BTW, I really appreciate the consideration you all have given to this issue!

[kenjis]

I forgot that we have improved the error message since v4.3.

$ php spark serve
PHP Fatal error:  Uncaught InvalidArgumentException: The file path contains special characters "~" that are not allowed: ".../~newApp/vendor/codeigniter4/framework/system/Autoloader/FileLocator.php" in /.../~newApp/vendor/codeigniter4/framework/system/Autoloader/Autoloader.php:342
Stack trace:

[kenjis]

In my opinion, the autoloader sanitization process should be removed. If we check, we should check only the classname.

Besides, I don't see the need for the public sanitizeFilename() method to exist in the autoloader.

[paulbalandan]

In my opinion, the autoloader sanitization process should be removed. If we check, we should check only the classname.

Besides, I don't see the need for the public sanitizeFilename() method to exist in the autoloader.

I agree. Looking at composer, I don't see being a security issue the absence of a sanitization method.

[zigmoo]

Hello All,

Are y'all feeling thumbs-up, or thumbs-down about the idea of changing Autoloader->sanitizeFilename() 's from its current state of disallowing all tildes in the path, to only negating a begins-with tilde?

$result = preg_match_all('/[^0-9\p{L}\s\/\-_.:\\\\]/u', $filename, $matches); to ++++ +
$result = preg_match_all('/^[~]|[^0-9\p{L}\s\/\-_.:\\\\~]/u', $filename, $matches);

Naturally, I would certainly appreciate this change.

[samsonasik]

I think add ~ is the safer, In Laminas, there is~ in URI check as well as unreserved char https://github.com/laminas/laminas-uri/blob/663b050294945c7345cc3a61f3ca661d5f9e1f80/src/Uri.php#L45

[kenjis]

This is not the case of URI. This is an issue in Autoloading.

I vote +1 to deprecate Autoloader->sanitizeFilename() and do not use it.

[kenjis]

I sent a PR #7310

[kenjis]

Closed by #7310