From 2654a1178e9cbbdd6230ed3cacef0760685880cf Mon Sep 17 00:00:00 2001 From: Philipp Wagner Date: Mon, 5 Oct 2026 22:08:02 +0200 Subject: [PATCH] Refuse a translation key that would not form a Kotlin name The generator escaped every translation value but emitted the key verbatim as the member identifier, and identifierFor strips only . _ - -- so a key carrying a space, a brace, a quote or a newline from a \uXXXX escape broke out of val : String get() = lookup(...) into an init block or property initializer that runs when Strings is instantiated. Translation files are the contribution surface that arrives by pull request, so this was a path from a merged .properties change to code in every client. The generated identifier is now held to letters-then-letters-and-digits before any source is written, alongside the existing orphan and collision refusals. Proven by replaying a file-writing payload against the built generator: it now throws and writes nothing. Co-Authored-By: Claude Opus 5 (1M context) --- .../kotlin/kst4contest/i18n/GeneratorMain.kt | 16 +++++++++++ .../kst4contest/i18n/TranslationRules.kt | 19 +++++++++++++ .../kotlin/kst4contest/i18n/GenerationTest.kt | 26 +++++++++++++++++ .../kst4contest/i18n/TranslationRulesTest.kt | 28 +++++++++++++++++++ 4 files changed, 89 insertions(+) diff --git a/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt b/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt index 4ab1a905..fe5b0767 100644 --- a/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt +++ b/i18n-generator/src/main/kotlin/kst4contest/i18n/GeneratorMain.kt @@ -50,6 +50,7 @@ fun generate(inputDirectory: File, outputDirectory: File): GenerationResult { + "language is where every key and every argument count comes from" ) + refuseIllegalIdentifiers(base) refuseIdentifierCollisions(base) val others = translations.filter { it.language != BASE_LANGUAGE } @@ -80,6 +81,21 @@ fun generate(inputDirectory: File, outputDirectory: File): GenerationResult { ) } +private fun refuseIllegalIdentifiers(base: Translation) { + val illegal = illegalIdentifierKeys(base.entries.keys) + + if (illegal.isEmpty()) { + return + } + + throw TranslationException( + "strings_$BASE_LANGUAGE.properties has keys that do not form a usable name -- a key is " + + "letters and digits with . _ - between words, nothing else. A key carrying " + + "anything more would be written into the generated source as code: " + + illegal.sorted().joinToString(", ") + ) +} + private fun refuseIdentifierCollisions(base: Translation) { val collisions = identifierCollisions(base.entries.keys) diff --git a/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt b/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt index a7dd1f81..9be4add2 100644 --- a/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt +++ b/i18n-generator/src/main/kotlin/kst4contest/i18n/TranslationRules.kt @@ -59,5 +59,24 @@ fun identifierFor(key: String): String { fun identifierCollisions(keys: Collection): Map> = keys.groupBy { identifierFor(it) }.filterValues { it.size > 1 } +/** + * Keys whose identifier would not be a legal Kotlin name. + * + * [identifierFor] strips only `.`, `_` and `-`; every other character a `Properties` file can + * carry in a key -- a space, a brace, a quote, a `=`, a newline decoded from a `\uXXXX` escape + * -- passes through into identifier position unchanged. Emitted there it does not merely fail + * to compile: the member is written as `val : String get() = lookup(...)`, so a + * key that reads `x: String get() = "" ... init { ... } ... val y` breaks out of the template + * and leaves a property initializer or an init block of the contributor's choosing, which + * runs when `Strings` is instantiated in the application. The value is already escaped before + * it becomes a literal; the identifier was not checked at all. So a generated name that is + * not letters-then-letters-and-digits is refused before any source is written. + */ +fun illegalIdentifierKeys(keys: Collection): List = + keys.filterNot { IDENTIFIER.matches(identifierFor(it)) } + +/** A letter, then letters and digits: the plain Kotlin identifier this generator emits. */ +private val IDENTIFIER = Regex("[A-Za-z][A-Za-z0-9]*") + /** `{0}`, `{12}`, `{0,number}` -- a brace, digits, then either a brace or a comma. */ private val PLACEHOLDER = Regex("""\{(\d+)\s*[,}]""") diff --git a/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt b/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt index dc3f4899..62b8a83c 100644 --- a/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt +++ b/i18n-generator/src/test/kotlin/kst4contest/i18n/GenerationTest.kt @@ -92,6 +92,32 @@ class GenerationTest { assertTrue(failure.message!!.contains("guiStartupDesign"), failure.message) } + @Test + fun aKeyCarryingKotlinSyntaxBreaksTheBuildRatherThanInjectingCode() { + /* + * The key, not the value, becomes the member name, and only . _ - are stripped from + * it. A \uXXXX escape lets a contributed .properties file carry a key that, dropped + * into `val : String get() = ...`, closes the template and opens a property + * initializer -- Kotlin that runs when Strings is built in every client. The base + * file is the vector: only base keys become members. This must fail the build the way + * an orphan or a collision does, not ship. + * + * The escapes below decode to the key `inject: Int = run{ pwn() }`. + */ + val (input, output) = directories() + write( + input, + "strings_en.properties", + "ok=fine\n" + + "inject\\u003a\\u0020Int\\u0020\\u003d\\u0020run\\u007b\\u0020pwn()\\u0020\\u007d=value\n", + ) + + val failure = assertThrows(TranslationException::class.java) { generate(input, output) } + + assertTrue(failure.message!!.contains("inject"), failure.message) + assertFalse(generated(output).exists(), "source was written despite an unusable key") + } + @Test fun aMissingBaseFileBreaksTheBuild() { val (input, output) = directories() diff --git a/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt b/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt index 02f842af..12dbcb85 100644 --- a/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt +++ b/i18n-generator/src/test/kotlin/kst4contest/i18n/TranslationRulesTest.kt @@ -103,4 +103,32 @@ class TranslationRulesTest { fun distinctKeysReportNoCollision() { assertTrue(identifierCollisions(listOf("a.b", "c.d")).isEmpty()) } + + @Test + fun aKeyThatWouldNotFormALegalIdentifierIsRejected() { + /* + * identifierFor strips only . _ -, so every other character a Properties file can + * carry in a key -- a space, a brace, a quote, a newline decoded from a \uXXXX escape + * -- would reach identifier position verbatim. Emitted there it does not merely fail + * to compile: it breaks out of `val : String get() = ...` into whatever the key + * says, which is Kotlin that then runs. So the generated name is held to the shape of + * an identifier before it is ever written. + */ + val illegal = illegalIdentifierKeys( + listOf("settings.save", "a b", "x\": String get() = \"\"", "init { pwn() }\n val") + ) + + assertEquals( + listOf("a b", "init { pwn() }\n val", "x\": String get() = \"\""), + illegal.sorted(), + ) + } + + @Test + fun ordinaryKeysFormLegalIdentifiersAndAreNotRejected() { + // A digit-leading key is fine: identifierFor prefixes it, which is a letter again. + assertTrue( + illegalIdentifierKeys(listOf("settings.save", "gui.startup_design", "2.metres")).isEmpty() + ) + } }