[2.1.x] open_basedir restriction warnings in php 8+

[isaak654]

Description

Derived from [2.0.19] open_basedir restriction with Altervista provider - this issue is only related to my findings with SMF 2.1.x and a different Altervista domain than the one reported.

Steps to reproduce

1. Create a new Altervista site for free from https://en.altervista.org/create-free-site.php ("Hosting with file manager" plan)
2. You may need to set PHP as version 8.0 or later from their control panel
3. Clean installation of SMF 2.1.3 (no mods)
4. Go to the Package Manager and go back and forth through the following tabs:
   
5. Multiple error log(s) like this will be produced:

/membri/AVforum2/Sources/Packages.php (Line 2308)
Backtrace information
Type of error: General
Error message Select
2: is_file(): open_basedir restriction in effect. File(/membri/AVforum2/..) is not within the allowed path(s): (/membri/AVforum2/:/membri2/AVforum2/:/membri/.dummy/)

Environment (complete as necessary)

• Version/Git revision: SMF 2.1.3
• Database: MySQL 8.0.26
• PHP Version: 8.0.26

Additional information/references

• open_basedir: /membri/AVforum2/:/membri2/AVforum2/:/membri/.dummy/
• php.ini can't be changed, even upon official request to the provider
• no apache log

[sbulen]

Note that we are only seeing these issues with Altervista, and not with other hosts, leading me to believe these are environment issues with how Altervista configures their free accounts.

I suspect there are two issues here. Not sure how to test/confirm without an account.

ISSUE 1: I believe that the format used in the open_basedir config, with closing slashes, means that no subfolders are allowed.

The package manager creates a temp folder to work within, and I don't think that is being allowed.

I.e.: /membri/AVforum2/:/membri2/AVforum2/:/membri/.dummy/
Needs to be: /membri/AVforum2:/membri2/AVforum2:/membri/.dummy

ISSUE 2: The temp folder is sometimes used by SMF. l.e., when open_basedir is in effect, it may also need to include the temp path as well.

I.e., if the temporary folder is /tmp, I think it needs to be: /membri/AVforum2:/membri2/AVforum2:/membri/.dummy:/tmp

[sbulen]

I'll see if I can replicate by adding open_basedir restrictions locally when I have time.

[sbulen]

Interesting...
php/php-src#8402

Note that some of these checks are, in fact, using a wildcard. All such checks now result in a basedir failure.

That's only part of the problem, though...

This is not restricted to Altervista alone.

Still digging...

[jdarwood007]

PHP likes to test all the way from root (/ or C:\ for example) through the entire path provided. So it will check to see if it has access to the \ path, but it does not (only a child path). I got around it by doing try catch statements

		try
		{
			set_error_handler(static function ($severity, $message, $file, $line) {
				throw new \ErrorException($message, 0, $severity, $file, $line);
			});

			[... CODE HERE ...]
		}
		catch (Exception $e)
		{
			if (strpos($e->getMessage(), 'open_basedir') == false)
				throw new \ErrorException($e->getMessage(), 0, $e->getSeverity(), $e->getFile(), $e->getLine());
		}
		finally
		{
			restore_error_handler();
		}

Not ideal, but there isn't a cleaner way to do it in PHP nowadays.

[sbulen]

At it's heart, this is a php 8.0 issue. open_basedir is throwing errors that it didn't before. If you go to php 7.4, everything works fine.

But... Why is it looking at places it shouldn't???

There are multiple paths of failure here, depending on the function. I have seen at least 2 thus far:

File Permissions: When entering File Permissions, it attempts to get data from all the files. It queries the file system. In the list, is ".." - which means go up a level. But... There it runs into a valid open_basedir failure - if open_basedir was set to the root folder where SMF is installed, it is properly being disallowed from looking up a level.

This is an easy fix - don't look at ".." as if it were a file. Change the order of the file checks.

This is the error reported above - Packages.php line 2308.

Browse Packages: When entering Browse Packages, it attempts to get all of the package-info.xml files for their contents. But... package-info.xml might be in a subfolder. So... it looks for "*/package-info.xml"... And runs into the php bug reported above.

It's not looking where it shouldn't here... Due to that php bug, ANY check on a file using a wildcard will cause an error if open_basedir is in effect.

EDIT... Part of the problem is that $destination is passed "*/package-info.xml". The $destination field is overloaded, sometimes it is the target folder, and when loading a single file (like package-info.xml), it identifies the file to load. The problem is it does the check that uses $destination as a folder first... I think it worked before because the is_file check just returned false and it moved along its merry way. But now it sees the wildcard and throws an error.

Probably solved by changing the order, just like the above.

This causes an error on Subs-Package.php line 162.

[sbulen]

PR submitted that addresses package installation, deinstallation, & file permissions functions.

Note that in my testing, I found that the temp folder needed to be specified in open_basedir, as I had warned above. SMF uses the temp folder for some things like uploading attachments. If your host did not include the temp folder in the open_basedir, that may hinder some functions, e.g., attachment uploads.

I did not encounter the other issue, i.e., where a trailing / meant no subfolders allowed. But I encountered it described out there when researching this issue, so bear that in mind.

I.e., open_basedir issues may be a mix of host config issues and SMF issues.

I am sure we will find other SMF issues with open_basedir. SMF tries to be very admin-friendly, and will often second guess the admin - "oh, the file's not here? let's see if we can find it over there.... oh, not there? let's try over there also!". The php 8+ open_basedir functions really really really don't like that kind of testing...

[dullboy3]

The temporary folder should be determined using sys_get_temp_dir() as /tmp may not be accessible, as regards Altervista provider the returned value in this case is /membri/.dummy/temp which is inside the open_basedir scope.

[isaak654]

/membri/.dummy/temp = session.save_path, so in theory the checks are done here.

The question is, is the code flawless enough?

[sbulen]

It sounds like the host did in fact include the temp folder in open_basedir - that's what the dummy entry is for.

So you should be covered.

Feel free to test the PR.

[Sesquipedalian]

The temporary folder should be determined using sys_get_temp_dir() as /tmp may not be accessible

Even better would be to use sm_temp_dir() in Subs-Admin.php, which has multiple fallbacks and checks for open_basedir restrictions.

[isaak654]

The changes implemented in the PR are fine, but there are more open_basedir restrictions to fix: #7741 (review)