diff --git a/lib/I18n.php b/lib/I18n.php
index 191b3753..ad2de03f 100644
--- a/lib/I18n.php
+++ b/lib/I18n.php
@@ -127,14 +127,13 @@ class I18n
} else {
$args[0] = self::$_translations[$messageId];
}
- // encode any non-integer arguments and the message ID, if it doesn't contain a link or keyboard input
+ // encode any non-integer arguments, but not the message itself
+ // The message ID comes from trusted sources (code or translation JSON files),
+ // while parameters may come from untrusted sources and need HTML entity encoding
+ // to prevent XSS attacks when the message is inserted into HTML context
$argsCount = count($args);
- for ($i = 0; $i < $argsCount; ++$i) {
- if ($i === 0) {
- if (str_contains($args[0], '')) {
- continue;
- }
- } elseif (is_int($args[$i])) {
+ for ($i = 1; $i < $argsCount; ++$i) {
+ if (is_int($args[$i])) {
continue;
}
$args[$i] = self::encode($args[$i]);
diff --git a/tst/I18nTest.php b/tst/I18nTest.php
index 7118c9fc..76511a25 100644
--- a/tst/I18nTest.php
+++ b/tst/I18nTest.php
@@ -38,6 +38,7 @@ class I18nTest extends TestCase
public function tearDown(): void
{
unset($_COOKIE['lang'], $_SERVER['HTTP_ACCEPT_LANGUAGE']);
+ I18n::loadTranslations();
}
public function testTranslationFallback()
@@ -183,7 +184,19 @@ class I18nTest extends TestCase
$result = htmlspecialchars($input, ENT_QUOTES | ENT_HTML5 | ENT_DISALLOWED, 'UTF-8', false);
$this->assertEquals($result, I18n::encode($input), 'encodes HTML entities');
$this->assertEquals('some ' . $result . ' + 1', I18n::_('some %s + %d', $input, 1), 'encodes parameters in translations');
- $this->assertEquals($result . $result, I18n::_($input . '%s', $input), 'encodes message ID as well, when no link');
+ // Message ID should NOT be encoded (it comes from trusted source), only the parameter should be
+ $this->assertEquals($input . $result, I18n::_($input . '%s', $input), 'encodes only parameters, not message ID');
+ }
+
+ public function testApostropheEncodngInMessage()
+ {
+ $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'fr';
+ I18n::loadTranslations();
+ // For example, the French translation should not have the apostrophe encoded
+ // See https://github.com/PrivateBin/PrivateBin/issues/1712
+ $message = I18n::_('Document does not exist, has expired or has been deleted.');
+ $this->assertFalse(strpos($message, ''') !== false, 'French apostrophe should not be encoded in translation message');
+ $this->assertTrue(strpos($message, "n'existe") !== false, 'French apostrophe should be present as literal character');
}
public function testFallbackAlwaysPresent()