From f07b35e1d3f08f8f8635e5331099ebeaf09b850f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metin=20=C5=9Eim=C5=9Fek?= <75851971+devsimsek@users.noreply.github.com> Date: Sun, 5 Jul 2026 18:05:30 +0200 Subject: [PATCH 1/2] fix(security): LocalDriver path traversal containment (SDF-57) Rewrite resolve() with lexical normalization - resolves . and .. segments without realpath() so it works for non-existent paths (put, stream, makeDirectory). Rejects null bytes and throws InvalidArgumentException when a path would escape the storage root. 5 new tests covering traversal in get/put/delete, null bytes, and legitimate .. normalization. Bump 2.3.1 -> 2.3.2. --- index.php | 2 +- sdf/__init.php | 2 +- sdf/core/Storage/Drivers/LocalDriver.php | 26 +++++++++++++++++++- tests/StorageTest.php | 31 ++++++++++++++++++++++++ wiki/libraries/storage.md | 19 +++++++++++++++ 5 files changed, 77 insertions(+), 3 deletions(-) diff --git a/index.php b/index.php index 58e409c..ec7a6e5 100644 --- a/index.php +++ b/index.php @@ -5,7 +5,7 @@ * Copyright devsimsek * @package SDF * @file index.php - * @version v2.3.1 + * @version v2.3.2 * @author devsimsek * @copyright Copyright (c) 2022 - 2026, smskSoft, devsimsek * @license https://opensource.org/licenses/MIT MIT License diff --git a/sdf/__init.php b/sdf/__init.php index d160ccb..49f80a0 100644 --- a/sdf/__init.php +++ b/sdf/__init.php @@ -22,7 +22,7 @@ print_r('PANIC: sdf is not called by it\'s own script.'); exit(1); } -const SDF_VERSION = "2.3.1"; +const SDF_VERSION = "2.3.2"; // Check minimum version requirement of this framework. // PHP 8.2 or higher is required, framework is tested and compatible up to PHP 8.5 diff --git a/sdf/core/Storage/Drivers/LocalDriver.php b/sdf/core/Storage/Drivers/LocalDriver.php index 396b25f..c72cdee 100644 --- a/sdf/core/Storage/Drivers/LocalDriver.php +++ b/sdf/core/Storage/Drivers/LocalDriver.php @@ -19,7 +19,31 @@ public function __construct(string $root, string $urlPrefix = '') private function resolve(string $path): string { - return $this->root . '/' . ltrim($path, '/'); + if (str_contains($path, "\0")) { + throw new \InvalidArgumentException('Storage path contains null byte'); + } + + $full = $this->root . '/' . ltrim($path, '/'); + $parts = explode('/', $full); + $normalized = []; + foreach ($parts as $part) { + if ($part === '' || $part === '.') { + continue; + } + if ($part === '..') { + array_pop($normalized); + continue; + } + $normalized[] = $part; + } + $normalizedPath = '/' . implode('/', $normalized); + + $rootPath = rtrim($this->root, '/'); + if (!str_starts_with($normalizedPath . '/', $rootPath . '/')) { + throw new \InvalidArgumentException('Path escapes storage root: ' . $path); + } + + return $normalizedPath; } public function exists(string $path): bool diff --git a/tests/StorageTest.php b/tests/StorageTest.php index 0aedc40..cdb83bf 100644 --- a/tests/StorageTest.php +++ b/tests/StorageTest.php @@ -191,6 +191,37 @@ public function test_url_no_prefix(): void $this->assertSame('file.jpg', $this->driver->url('file.jpg')); } + public function test_path_traversal_in_get_throws(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->driver->get('../../../etc/passwd'); + } + + public function test_path_traversal_in_put_throws(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->driver->put('../../outside.txt', 'evil'); + } + + public function test_path_traversal_in_delete_throws(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->driver->delete('../../../etc/passwd'); + } + + public function test_null_byte_in_path_throws(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->driver->get("file\0.txt"); + } + + public function test_dotdot_inside_path_normalizes(): void + { + $this->driver->makeDirectory('sub'); + $this->driver->put('sub/file.txt', 'inside'); + $this->assertSame('inside', $this->driver->get('sub/../sub/file.txt')); + } + public function test_facade_put_and_exists(): void { $this->setUpStorageConfig(); diff --git a/wiki/libraries/storage.md b/wiki/libraries/storage.md index 31d7ecb..2d5a3fc 100644 --- a/wiki/libraries/storage.md +++ b/wiki/libraries/storage.md @@ -68,6 +68,25 @@ $driver->makeDirectory('new/path'); $driver->deleteDirectory('old/path'); ``` +### Path traversal protection (v2.3.2+) + +As of **v2.3.2**, every `LocalDriver` method runs its `$path` through a lexical normalizer before touching the filesystem. The normalizer: + +- Rejects null bytes (`\0`) with `InvalidArgumentException` +- Resolves `.` and `..` segments purely lexically (no `realpath()` needed, so it works for files that don't exist yet) +- Asserts the resolved path stays inside the configured root + +If a path would escape the root, an `InvalidArgumentException` is thrown: + +```php +$driver->get('../../../etc/passwd'); // throws InvalidArgumentException +$driver->put('../../shell.php', 'get("file\0.txt"); // throws InvalidArgumentException +$driver->get('sub/../sub/file.txt'); // OK - normalizes to 'sub/file.txt' +``` + +This applies to all methods: `get`, `put`, `stream`, `delete`, `exists`, `size`, `mimeType`, `files`, `allFiles`, `directories`, `copy`, `move`, `makeDirectory`, `deleteDirectory`. + ## S3Driver Requires `aws/aws-sdk-php` via Composer. Supports S3-compatible object stores (MinIO, DigitalOcean Spaces, etc.) via the `endpoint` config key. From 5a2c1bd32c55135ac9b3f6ec18bed01e83805887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metin=20=C5=9Eim=C5=9Fek?= <75851971+devsimsek@users.noreply.github.com> Date: Sun, 5 Jul 2026 18:12:11 +0200 Subject: [PATCH 2/2] fix(security): FileDriver allowed_classes=false (SDF-59) Switch unserialize from allowed_classes=true to false, blocking object-injection RCE gadget chains. Brings FileDriver in line with RedisDriver and MemcachedDriver. Breaking for users caching objects - documented migration path in wiki. Bump 2.3.2 -> 2.3.3. --- index.php | 2 +- sdf/__init.php | 2 +- sdf/core/Cache/FileDriver.php | 2 +- tests/CacheTest.php | 16 ++++++++++++++++ wiki/libraries/caching.md | 22 ++++++++++++++++++++++ 5 files changed, 41 insertions(+), 3 deletions(-) diff --git a/index.php b/index.php index ec7a6e5..1b71c4b 100644 --- a/index.php +++ b/index.php @@ -5,7 +5,7 @@ * Copyright devsimsek * @package SDF * @file index.php - * @version v2.3.2 + * @version v2.3.3 * @author devsimsek * @copyright Copyright (c) 2022 - 2026, smskSoft, devsimsek * @license https://opensource.org/licenses/MIT MIT License diff --git a/sdf/__init.php b/sdf/__init.php index 49f80a0..2404a76 100644 --- a/sdf/__init.php +++ b/sdf/__init.php @@ -22,7 +22,7 @@ print_r('PANIC: sdf is not called by it\'s own script.'); exit(1); } -const SDF_VERSION = "2.3.2"; +const SDF_VERSION = "2.3.3"; // Check minimum version requirement of this framework. // PHP 8.2 or higher is required, framework is tested and compatible up to PHP 8.5 diff --git a/sdf/core/Cache/FileDriver.php b/sdf/core/Cache/FileDriver.php index f035f1c..4097cdf 100644 --- a/sdf/core/Cache/FileDriver.php +++ b/sdf/core/Cache/FileDriver.php @@ -262,7 +262,7 @@ private function read(string $key): ?array if ($content === false) { return null; } - $data = unserialize($content, ['allowed_classes' => true]); + $data = unserialize($content, ['allowed_classes' => false]); if (!is_array($data)) { return null; } diff --git a/tests/CacheTest.php b/tests/CacheTest.php index 776114a..5edc648 100644 --- a/tests/CacheTest.php +++ b/tests/CacheTest.php @@ -121,6 +121,22 @@ public function test_file_driver_handles_serialized_types(): void $this->assertSame($data, $driver->get('complex')); } + public function test_file_driver_refuses_object_unserialization(): void + { + $driver = new FileDriver(['path' => $this->tmpDir, 'prefix' => 'test_']); + + // Craft a serialized object payload that would trigger __wakeup on a + // vulnerable class. Simulate an attacker writing the cache file directly. + $payload = 'O:8:"stdClass":1:{s:3:"foo";s:3:"bar";}'; + $file = $this->tmpDir . '/test_obj.cache'; + file_put_contents($file, $payload); + + // With allowed_classes=false, unserialize returns __PHP_Incomplete_Class + // which fails the is_array() check and returns null - no gadget chain fires. + $result = $driver->get('obj'); + $this->assertNull($result); + } + // ─── FileDriver: Tagging ────────────────────────────────────────────────── public function test_file_driver_tagging(): void diff --git a/wiki/libraries/caching.md b/wiki/libraries/caching.md index 371e8b2..5e7d536 100644 --- a/wiki/libraries/caching.md +++ b/wiki/libraries/caching.md @@ -112,6 +112,28 @@ Cache::normalizeKey(''); // throws \InvalidArgumentException > If you cache objects, note that `FileDriver` currently unserializes with `allowed_classes => true`. Keep your cache directory private (`app/cache/` rather than `/tmp`) and never cache content that could be tampered with by another tenant. A future release will switch the default to `allowed_classes => false` for defense in depth. +### Object unserialization safety (v2.3.3+) + +As of **v2.3.3**, `FileDriver::read()` unserializes with `allowed_classes => false`. This means **cached objects are no longer resurrected into PHP class instances** - they become `__PHP_Incomplete_Class` instead, which is a safe no-op (no `__wakeup`/`__destruct` magic methods fire). This blocks object-injection RCE gadget chains (Monolog, Guzzle, etc.) even if an attacker can write to the cache directory. + +**Breaking change for users caching objects:** if you previously did `Cache::set('user', $userObject)` and expected `Cache::get('user')` to return a `User` instance, you will now get `null` (because the incomplete class fails the `is_array()` check) or an `__PHP_Incomplete_Class` if you bypass the facade. **Migrate to caching arrays/scalars:** + +```php +// Before (v2.3.2 and earlier) +Cache::set('user', $userObject); +$user = Cache::get('user'); // User instance + +// After (v2.3.3+) +Cache::set('user', $userObject->toArray()); +$user = Cache::get('user'); // array + +// Or JSON-encode explicitly +Cache::set('user', json_encode($userObject)); +$user = User::fromArray(json_decode(Cache::get('user'), true)); +``` + +This brings `FileDriver` in line with `RedisDriver` and `MemcachedDriver`, which already used `allowed_classes => false`. The tag-index unserializer (`loadTagIndex()`) was already safe. + ## Drivers | Driver | Extension required | Storage |