diff options
| author | 2026-01-03 18:09:44 +0100 | |
|---|---|---|
| committer | 2026-01-03 18:09:44 +0100 | |
| commit | 26c1102567c095b051b5e1a0aedb45b78713c283 (patch) | |
| tree | 0c6d68799b7ff55019cc7ee3e73ea38087193006 | |
| parent | 15814cfd35b8ac704a761530e14bd9efe6500ddc (diff) | |
Merge commit from fork
* Fix Path Traversal vulnerability in UserDAO methods
* Add tests and changelog for UserDAO path traversal fix
* make fix-all
* Fix PHPStan
---------
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | app/Models/UserDAO.php | 9 | ||||
| -rw-r--r-- | tests/app/Models/UserDAOTest.php | 78 |
3 files changed, 88 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 6706d66ef..e5341328d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ See also [the FreshRSS releases](https://github.com/FreshRSS/FreshRSS/releases). * Disable counting articles in user labels for Ajax requests (unused) [#8352](https://github.com/FreshRSS/FreshRSS/pull/8352) * Security * Change `Content-Disposition: inline` to `attachment` in `f.php` [#8344](https://github.com/FreshRSS/FreshRSS/pull/8344) + * Fix Path Traversal vulnerability in `UserDAO` methods (`exists`, `mtime`, `ctime`) [GHSA-p8fh-pp43-9372](https://github.com/FreshRSS/FreshRSS/security/advisories/GHSA-p8fh-pp43-9372) * Extensions * Update `.gitignore` to ignore installed extensions [#8372](https://github.com/FreshRSS/FreshRSS/pull/8372) * Misc. diff --git a/app/Models/UserDAO.php b/app/Models/UserDAO.php index 89f8f2a77..d61b5d9c9 100644 --- a/app/Models/UserDAO.php +++ b/app/Models/UserDAO.php @@ -49,6 +49,9 @@ class FreshRSS_UserDAO extends Minz_ModelPdo { } public static function exists(string $username): bool { + if (!FreshRSS_user_Controller::checkUsername($username)) { + return false; + } return is_dir(USERS_PATH . '/' . $username); } @@ -64,6 +67,9 @@ class FreshRSS_UserDAO extends Minz_ModelPdo { /** Time of the last modification action by the user (e.g., mark an article as read) */ public static function mtime(string $username): int { + if (!FreshRSS_user_Controller::checkUsername($username)) { + return 0; + } return @filemtime(USERS_PATH . '/' . $username . '/config.php') ?: 0; } @@ -79,6 +85,9 @@ class FreshRSS_UserDAO extends Minz_ModelPdo { /** Time of the last new content automatically received by the user (e.g., cron job, WebSub) */ public static function ctime(string $username): int { + if (!FreshRSS_user_Controller::checkUsername($username)) { + return 0; + } return @filemtime(USERS_PATH . '/' . $username . '/' . LOG_FILENAME) ?: 0; } } diff --git a/tests/app/Models/UserDAOTest.php b/tests/app/Models/UserDAOTest.php new file mode 100644 index 000000000..33a59c504 --- /dev/null +++ b/tests/app/Models/UserDAOTest.php @@ -0,0 +1,78 @@ +<?php +declare(strict_types=1); + +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; + +class UserDAOTest extends TestCase { + + #[DataProvider('pathTraversalPayloadsProvider')] + public function testExistsRejectsPathTraversal(string $payload): void { + self::assertFalse(FreshRSS_UserDAO::exists($payload)); + } + + #[DataProvider('pathTraversalPayloadsProvider')] + public function testMtimeRejectsPathTraversal(string $payload): void { + self::assertSame(0, FreshRSS_UserDAO::mtime($payload)); + } + + #[DataProvider('pathTraversalPayloadsProvider')] + public function testCtimeRejectsPathTraversal(string $payload): void { + self::assertSame(0, FreshRSS_UserDAO::ctime($payload)); + } + + /** + * @return array<string,array<int,string>> + */ + public static function pathTraversalPayloadsProvider(): array { + return [ + 'parent directory' => ['../'], + 'double parent directory' => ['../../'], + 'traversal to app' => ['../../app'], + 'traversal to etc' => ['../../../etc'], + 'traversal with null byte' => ["../\0"], + 'absolute path' => ['/etc/passwd'], + 'dot only' => ['.'], + 'double dot' => ['..'], + 'slash in name' => ['user/config'], + 'backslash traversal' => ['..\\..\\app'], + 'encoded traversal' => ['%2e%2e%2f'], + 'mixed traversal' => ['valid/../invalid'], + 'empty string' => [''], + ]; + } + + #[DataProvider('validUsernamesProvider')] + public function testExistsAcceptsValidUsernames(string $username): void { + $result = FreshRSS_UserDAO::exists($username); + self::assertIsBool($result); + } + + #[DataProvider('validUsernamesProvider')] + public function testMtimeAcceptsValidUsernames(string $username): void { + $result = FreshRSS_UserDAO::mtime($username); + self::assertIsInt($result); + } + + #[DataProvider('validUsernamesProvider')] + public function testCtimeAcceptsValidUsernames(string $username): void { + $result = FreshRSS_UserDAO::ctime($username); + self::assertIsInt($result); + } + + /** + * @return array<string,array<int,string>> + */ + public static function validUsernamesProvider(): array { + return [ + 'simple' => ['alice'], + 'with numbers' => ['user123'], + 'with underscore' => ['test_user'], + 'with dot' => ['user.name'], + 'with hyphen' => ['user-name'], + 'with at' => ['user@domain'], + 'single char' => ['a'], + 'max length' => [str_repeat('a', 39)], + ]; + } +} |
