diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php index 27d29eb3b356e..fa8cc51f60636 100644 --- a/lib/private/L10N/Factory.php +++ b/lib/private/L10N/Factory.php @@ -94,11 +94,8 @@ public function __construct( public function get($app, $lang = null, $locale = null) { return new LazyL10N(function () use ($app, $lang, $locale) { $app = $this->appManager->cleanAppId($app); - if ($lang !== null) { - $lang = str_replace(['\0', '/', '\\', '..'], '', $lang); - } - - $forceLang = $this->request->getParam('forceLanguage') ?? $this->config->getSystemValue('force_language', false); + $lang = $this->cleanLanguage($lang); + $forceLang = $this->cleanLanguage($this->request->getParam('forceLanguage')) ?? $this->config->getSystemValue('force_language', false); if (is_string($forceLang)) { $lang = $forceLang; } @@ -128,6 +125,29 @@ public function get($app, $lang = null, $locale = null) { }); } + /** + * Remove some invalid characters before using a string as a language + * + * @psalm-taint-escape callable + * @psalm-taint-escape cookie + * @psalm-taint-escape file + * @psalm-taint-escape has_quotes + * @psalm-taint-escape header + * @psalm-taint-escape html + * @psalm-taint-escape include + * @psalm-taint-escape ldap + * @psalm-taint-escape shell + * @psalm-taint-escape sql + * @psalm-taint-escape unserialize + */ + private function cleanLanguage(?string $lang): ?string { + if ($lang === null) { + return null; + } + $lang = preg_replace('/[^a-zA-Z0-9.;,=-]/', '', $lang); + return str_replace('..', '', $lang); + } + /** * Check that $lang is an existing language and not null, otherwise return the language to use instead * @@ -160,7 +180,7 @@ private function validateLanguage(string $app, ?string $lang): string { */ public function findLanguage(?string $appId = null): string { // Step 1: Forced language always has precedence over anything else - $forceLang = $this->request->getParam('forceLanguage') ?? $this->config->getSystemValue('force_language', false); + $forceLang = $this->cleanLanguage($this->request->getParam('forceLanguage')) ?? $this->config->getSystemValue('force_language', false); if (is_string($forceLang)) { $this->requestLanguage = $forceLang; } @@ -217,7 +237,7 @@ public function findLanguage(?string $appId = null): string { public function findGenericLanguage(?string $appId = null): string { // Step 1: Forced language always has precedence over anything else - $forcedLanguage = $this->request->getParam('forceLanguage') ?? $this->config->getSystemValue('force_language', false); + $forcedLanguage = $this->cleanLanguage($this->request->getParam('forceLanguage')) ?? $this->config->getSystemValue('force_language', false); if ($forcedLanguage !== false) { return $forcedLanguage; } @@ -412,7 +432,8 @@ public function getUserLanguage(?IUser $user = null): string { return $language; } - if (($forcedLanguage = $this->request->getParam('forceLanguage')) !== null) { + $forcedLanguage = $this->cleanLanguage($this->request->getParam('forceLanguage')); + if ($forcedLanguage !== null) { return $forcedLanguage; } @@ -426,7 +447,7 @@ public function getUserLanguage(?IUser $user = null): string { } } - return $this->request->getParam('forceLanguage') ?? $this->config->getSystemValueString('default_language', 'en'); + return $this->cleanLanguage($this->request->getParam('forceLanguage')) ?? $this->config->getSystemValueString('default_language', 'en'); } /** @@ -452,7 +473,7 @@ public function localeExists($locale) { * @throws LanguageNotFoundException */ private function getLanguageFromRequest(?string $app = null): string { - $header = $this->request->getHeader('ACCEPT_LANGUAGE'); + $header = $this->cleanLanguage($this->request->getHeader('ACCEPT_LANGUAGE')); if ($header !== '') { $available = $this->findAvailableLanguages($app); diff --git a/tests/lib/L10N/FactoryTest.php b/tests/lib/L10N/FactoryTest.php index f83dd0bbf8904..bb43c68a21925 100644 --- a/tests/lib/L10N/FactoryTest.php +++ b/tests/lib/L10N/FactoryTest.php @@ -20,6 +20,7 @@ use OCP\IUser; use OCP\IUserSession; use OCP\L10N\ILanguageIterator; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -90,6 +91,21 @@ protected function getFactory(array $methods = [], $mockRequestGetHeaderMethod = return new Factory($this->config, $this->request, $this->userSession, $this->cacheFactory, $this->serverRoot, $this->appManager); } + public static function dataCleanLanguage(): array { + return [ + 'null shortcut' => [null, null], + 'default language' => ['de', 'de'], + 'malicious language' => ['de/../fr', 'defr'], + 'request language' => ['kab;q=0.8,ka;q=0.7,de;q=0.6', 'kab;q=0.8,ka;q=0.7,de;q=0.6'], + ]; + } + + #[DataProvider('dataCleanLanguage')] + public function testCleanLanguage(?string $lang, ?string $expected): void { + $factory = $this->getFactory(); + $this->assertSame($expected, self::invokePrivate($factory, 'cleanLanguage', [$lang])); + } + public static function dataFindAvailableLanguages(): array { return [ [null],