Path::isOwner() hardcodes /tmp instead of using sys_get_temp_dir()#80
Open
manusfreedom wants to merge 1 commit into
Open
Path::isOwner() hardcodes /tmp instead of using sys_get_temp_dir()#80manusfreedom wants to merge 1 commit into
Path::isOwner() hardcodes /tmp instead of using sys_get_temp_dir()#80manusfreedom wants to merge 1 commit into
Conversation
# Bug Report: `Path::isOwner()` hardcodes `/tmp` instead of using `sys_get_temp_dir()`
**Component:** `libraries/vendor/joomla/filesystem/src/Path.php`
**Method:** `Path::isOwner()`
**Affects:** Joomla 4.x / 5.x
> **Note:** The content of this bug report and the proposed fix were AI-assisted (generated with Claude). The root cause, reproduction steps, and the process of identifying the issue were found through real investigation and debugging — only the write-up itself was generated with AI assistance.
---
## Summary
`Path::isOwner()` hardcodes `/tmp` as the first candidate for a writable temporary directory, instead of using PHP's `sys_get_temp_dir()`. This causes failures on systems where the PHP temporary directory is configured differently — such as environments with AppArmor, SELinux, `open_basedir` restrictions, or `PrivateTmp=true` in systemd — where `/tmp` may be inaccessible to the php-fpm process.
---
## Steps to Reproduce
1. Configure php-fpm with a custom temporary directory (e.g. `sys_temp_dir = /var/lib/php/tmp` in `php.ini` or pool config)
2. Restrict access to `/tmp` via AppArmor, SELinux, or `open_basedir`
3. Save Global Configuration in Joomla administrator
4. `Path::isOwner()` is called, attempts `is_writable('/tmp')`, fails, falls through to `.` (current directory) or `session.save_path`
5. AppArmor logs `DENIED operation="mknod"` on `/tmp/<hash>`
6. The UI displays: `Joomla\Filesystem\File::delete: Failed deleting <hash>`
---
## Root Cause
```php
// Current — hardcoded /tmp, ignores PHP configuration
$dir = is_writable('/tmp') ? '/tmp' : false;
```
PHP provides `sys_get_temp_dir()` precisely to abstract the platform/configuration-specific temp directory. It correctly reflects:
- `sys_temp_dir` from `php.ini`
- `upload_tmp_dir` fallback
- Pool-level `php_value[sys_temp_dir]` overrides
- OS-level temp dir on Windows (`%TEMP%`) and non-standard Linux setups
---
## Proposed Fix
```php
// Fixed — respects PHP configuration
$dir = is_writable(sys_get_temp_dir()) ? sys_get_temp_dir() : false;
```
Full corrected method:
```php
/**
* Method to determine if script owns the path.
*
* @param string $path Path to check ownership.
*
* @return boolean True if the php script owns the path passed.
*
* @SInCE 1.0
*/
public static function isOwner($path)
{
$tmp = md5(random_bytes(16));
$ssp = ini_get('session.save_path');
// Use PHP's configured temp dir instead of hardcoded /tmp
$dir = is_writable(sys_get_temp_dir()) ? sys_get_temp_dir() : false;
$dir = !$dir && is_writable('.') ? '.' : $dir;
$dir = !$dir && is_writable($ssp) ? $ssp : $dir;
if ($dir) {
$test = $dir . '/' . $tmp;
$blank = '';
File::write($test, $blank, false);
$return = fileowner($test) === fileowner($path);
File::delete($test);
return $return;
}
return false;
}
```
---
## Impact
| Area | Description |
|---|---|
| **Security** | Hardcoding `/tmp` forces administrators to grant php-fpm access to the system `/tmp`, undermining AppArmor/SELinux confinement and `PrivateTmp=true` isolation |
| **Correctness** | Ignores explicit PHP configuration (`sys_temp_dir`, pool-level overrides) |
| **Portability** | Breaks on Windows and non-standard Linux setups where `/tmp` does not exist |
---
## Environment
- Joomla 4.x / 5.x
- php-fpm with AppArmor profile (`PrivateTmp=true` or custom `sys_temp_dir`)
- File: `libraries/vendor/joomla/filesystem/src/Path.php`
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates temporary directory detection to use PHP’s configured temp directory instead of hard-coding /tmp, improving portability across environments.
Changes:
- Replace
/tmpcheck withsys_get_temp_dir()inPath::isOwner().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug Report:
Path::isOwner()hardcodes/tmpinstead of usingsys_get_temp_dir()Component:
libraries/vendor/joomla/filesystem/src/Path.phpMethod:Path::isOwner()Affects: Joomla 4.x / 5.x
Summary
Path::isOwner()hardcodes/tmpas the first candidate for a writable temporary directory, instead of using PHP'ssys_get_temp_dir(). This causes failures on systems where the PHP temporary directory is configured differently — such as environments with AppArmor, SELinux,open_basedirrestrictions, orPrivateTmp=truein systemd — where/tmpmay be inaccessible to the php-fpm process.Steps to Reproduce
sys_temp_dir = /var/lib/php/tmpinphp.inior pool config)/tmpvia AppArmor, SELinux, oropen_basedirPath::isOwner()is called, attemptsis_writable('/tmp'), fails, falls through to.(current directory) orsession.save_pathDENIED operation="mknod"on/tmp/<hash>Joomla\Filesystem\File::delete: Failed deleting <hash>Root Cause
PHP provides
sys_get_temp_dir()precisely to abstract the platform/configuration-specific temp directory. It correctly reflects:sys_temp_dirfromphp.iniupload_tmp_dirfallbackphp_value[sys_temp_dir]overrides%TEMP%) and non-standard Linux setupsProposed Fix
Full corrected method:
Impact
/tmpforces administrators to grant php-fpm access to the system/tmp, undermining AppArmor/SELinux confinement andPrivateTmp=trueisolationEnvironment
PrivateTmp=trueor customsys_temp_dir)libraries/vendor/joomla/filesystem/src/Path.phpPull Request for Issue #
Summary of Changes
Testing Instructions
Documentation Changes Required