summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Bartłomiej Dmitruk <bartek.dmitruk@gmail.com> 2026-01-03 18:09:44 +0100
committerGravatar GitHub <noreply@github.com> 2026-01-03 18:09:44 +0100
commit26c1102567c095b051b5e1a0aedb45b78713c283 (patch)
tree0c6d68799b7ff55019cc7ee3e73ea38087193006
parent15814cfd35b8ac704a761530e14bd9efe6500ddc (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.md1
-rw-r--r--app/Models/UserDAO.php9
-rw-r--r--tests/app/Models/UserDAOTest.php78
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)],
+ ];
+ }
+}