diff --git a/lib/src/main/cpp/Terminal.cpp b/lib/src/main/cpp/Terminal.cpp index b727c1b1..6b8fe72f 100644 --- a/lib/src/main/cpp/Terminal.cpp +++ b/lib/src/main/cpp/Terminal.cpp @@ -1066,8 +1066,17 @@ void Terminal::invokeKeyboardOutput(const char* data, size_t len) { } int Terminal::invokeOscSequence(int command, const std::string& payload, int cursorRow, int cursorCol) { - LOGD("invokeOscSequence: command=%d, payload='%s' (len=%zu), cursor=(%d,%d)", - command, payload.c_str(), payload.length(), cursorRow, cursorCol); + // The payload is NEVER logged. It is arbitrary data from the remote end and + // routinely carries secrets: OSC 52 is the clipboard, so termSelectionSet + // hands the *decoded* copied text through here — a password out of a + // manager, a token, a private key — and OSC 3008 carries user, hostname and + // cwd. Logging it wrote all of that to logcat in plaintext, where adb + // logcat, a bug report, or a user pasting a log into an issue picks it up. + // + // Command and length are kept: they are what makes the log useful for + // sequencing and truncation bugs, and neither reveals content. + LOGD("invokeOscSequence: command=%d, payload len=%zu, cursor=(%d,%d)", + command, payload.length(), cursorRow, cursorCol); if (!mOscSequenceMethod) { LOGE("invokeOscSequence: mOscSequenceMethod is null"); @@ -1080,7 +1089,19 @@ int Terminal::invokeOscSequence(int command, const std::string& payload, int cur return 0; } - ScopedLocalRef payloadStr(env, env->NewStringUTF(payload.c_str())); + // Decoded to UTF-16 rather than handed to NewStringUTF, which aborts the + // whole process on bytes that are not valid modified UTF-8. An OSC payload + // is whatever the remote program emitted — a window title from a non-UTF-8 + // Windows console is not obliged to be valid UTF-8, and a terminal that + // dies on one is worse than a terminal that shows U+FFFD. + // + // Sized from the string rather than c_str(), so a payload containing an + // embedded NUL is passed through whole instead of being silently truncated. + const std::u16string payloadUtf16 = utf8_to_utf16_lossy(payload.data(), payload.size()); + ScopedLocalRef payloadStr( + env, + env->NewString(reinterpret_cast(payloadUtf16.data()), + static_cast(payloadUtf16.size()))); if (!payloadStr.get()) { LOGE("Failed to create jstring for OSC payload"); return 0; diff --git a/lib/src/main/cpp/mutf8.cpp b/lib/src/main/cpp/mutf8.cpp index 36a1a7ea..660b6741 100644 --- a/lib/src/main/cpp/mutf8.cpp +++ b/lib/src/main/cpp/mutf8.cpp @@ -186,3 +186,75 @@ char* mutf8_to_utf8(const char* mutf8_in, size_t len, size_t* out_len) { return utf8_out; } + +/** Append one code point as UTF-16, folding anything unencodable to U+FFFD. */ +static void append_code_point(std::u16string& out, uint32_t cp) { + // Lone surrogates are not encodable and would produce an invalid jstring. + if (cp > 0x10FFFF || (cp >= 0xD800 && cp <= 0xDFFF)) { + cp = 0xFFFD; + } + if (cp < 0x10000) { + out.push_back(static_cast(cp)); + } else { + cp -= 0x10000; + out.push_back(static_cast(0xD800 + (cp >> 10))); + out.push_back(static_cast(0xDC00 + (cp & 0x3FF))); + } +} + +std::u16string utf8_to_utf16_lossy(const char* in, size_t len) { + std::u16string out; + if (!in) return out; + out.reserve(len); + + const uint8_t* p = reinterpret_cast(in); + const uint8_t* end = p + len; + + while (p < end) { + const uint8_t c = *p; + uint32_t cp; + int continuations; + uint32_t smallest; // guards against overlong encodings + + if (c < 0x80) { + cp = c; continuations = 0; smallest = 0; + } else if ((c & 0xE0) == 0xC0) { + cp = c & 0x1Fu; continuations = 1; smallest = 0x80; + } else if ((c & 0xF0) == 0xE0) { + cp = c & 0x0Fu; continuations = 2; smallest = 0x800; + } else if ((c & 0xF8) == 0xF0) { + cp = c & 0x07u; continuations = 3; smallest = 0x10000; + } else { + // A stray continuation byte, or 0xF8..0xFF which UTF-8 never uses. + out.push_back(0xFFFD); + p++; + continue; + } + + // Truncated at the end of the buffer — this is the check whose absence + // makes utf8_to_mutf8 read past its input. + if (static_cast(end - p) < static_cast(continuations) + 1) { + out.push_back(0xFFFD); + p++; + continue; + } + + bool wellFormed = true; + for (int i = 1; i <= continuations; i++) { + if ((p[i] & 0xC0) != 0x80) { wellFormed = false; break; } + cp = (cp << 6) | (p[i] & 0x3Fu); + } + // Advance one byte on a bad sequence so the next lead byte gets its own + // chance to parse, rather than swallowing the rest of the sequence. + if (!wellFormed || cp < smallest) { + out.push_back(0xFFFD); + p++; + continue; + } + + append_code_point(out, cp); + p += continuations + 1; + } + + return out; +} diff --git a/lib/src/main/cpp/mutf8.h b/lib/src/main/cpp/mutf8.h index 4c79921e..b6afd0b8 100644 --- a/lib/src/main/cpp/mutf8.h +++ b/lib/src/main/cpp/mutf8.h @@ -17,6 +17,7 @@ #include #include +#include #ifndef CB_TERM_MUTF8_H #define CB_TERM_MUTF8_H @@ -24,4 +25,23 @@ char* utf8_to_mutf8(const char* utf8_in, size_t len, size_t* out_len); char* mutf8_to_utf8(const char* mutf8_in, size_t len, size_t* out_len); +/** + * Decode UTF-8 into UTF-16, substituting U+FFFD for anything malformed. + * + * JNI's NewStringUTF aborts the whole process when handed bytes that are not + * valid modified UTF-8, so it must never see untrusted input. An OSC payload is + * whatever the remote program chose to emit — from a non-UTF-8 Windows console + * that need not be valid UTF-8 at all. NewString takes UTF-16 and has no such + * failure mode, so decoding here removes the abort rather than narrowing the + * window for it. + * + * Lossy on purpose: a terminal that shows a replacement character for a + * mis-encoded title is behaving correctly; one that kills the app is not. + * + * Unlike utf8_to_mutf8 this makes no assumption that the input is well-formed — + * every multi-byte sequence is length-checked against the end of the buffer + * before its continuation bytes are read. + */ +std::u16string utf8_to_utf16_lossy(const char* in, size_t len); + #endif //CB_TERM_MUTF8_H diff --git a/lib/src/test/java/org/connectbot/terminal/OscMalformedUtf8Test.kt b/lib/src/test/java/org/connectbot/terminal/OscMalformedUtf8Test.kt new file mode 100644 index 00000000..8d4b2b11 --- /dev/null +++ b/lib/src/test/java/org/connectbot/terminal/OscMalformedUtf8Test.kt @@ -0,0 +1,194 @@ +/* + * ConnectBot Terminal + * Copyright 2025 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.connectbot.terminal + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +/** + * An OSC payload is whatever the remote program chose to emit, and it is not + * obliged to be valid UTF-8 — a window title from a non-UTF-8 Windows console + * is the everyday case. + * + * `invokeOscSequence` used to hand those bytes straight to `NewStringUTF`, which + * does not fail politely: ART treats invalid modified UTF-8 as a fatal error and + * aborts the process. There is no exception to catch and nothing the Kotlin side + * can do about it — the app is simply gone. + * + * ONE THING THESE TESTS DO NOT PROVE. They run on the host JVM, whose + * NewStringUTF is lenient about malformed modified UTF-8 — it returns mojibake + * rather than aborting. Only ART aborts. Measured: with the fix reverted, every + * test here still passes. So the liveness tests below cannot catch the abort and + * must not be read as proof against it; that regression is only reproducible on + * a device. + * + * What IS falsifiable on the host is the decode itself — malformed bytes must + * come out as U+FFFD rather than being passed through raw. That is + * [malformedBytesBecomeReplacementCharacters], and it does fail against the old + * NewStringUTF path. + * + * Note the truncated four-byte lead below. That case also walks off the end of + * the input in `utf8_to_mutf8`, which reads `p[1..3]` unconditionally — the + * reason this fix decodes to UTF-16 rather than routing through that helper. + */ +@RunWith(AndroidJUnit4::class) +class OscMalformedUtf8Test { + + /** Byte sequences that are not valid UTF-8, each malformed a different way. */ + private val malformedPayloads = listOf( + "lone continuation byte" to byteArrayOf(0x80.toByte()), + "truncated 2-byte lead" to byteArrayOf(0xC3.toByte()), + "truncated 3-byte lead" to byteArrayOf(0xE2.toByte(), 0x82.toByte()), + "truncated 4-byte lead" to byteArrayOf(0xF0.toByte(), 0x9F.toByte()), + "bytes UTF-8 never uses" to byteArrayOf(0xFF.toByte(), 0xFE.toByte()), + "encoded surrogate" to byteArrayOf(0xED.toByte(), 0xA0.toByte(), 0x80.toByte()), + "overlong NUL" to byteArrayOf(0xC0.toByte(), 0x80.toByte()), + "continuation without lead, mid-text" to + "ok".toByteArray() + byteArrayOf(0xBF.toByte()) + "ok".toByteArray(), + ) + + /** ESC ] ; BEL — without the ESC it is not an OSC at all. */ + private fun osc(command: String, payload: ByteArray): ByteArray = "\u001B]$command;".toByteArray() + payload + byteArrayOf(0x07) + + @Test + fun malformedOscPayloadsDoNotAbortTheProcess() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 24, initialCols = 80) + + for ((_, payload) in malformedPayloads) { + // OSC 0 is the window title, the sequence most likely to carry text + // straight from the remote host's locale. + emulator.writeInput(osc("0", payload)) + delay(10) + } + + // Whatever those did, the terminal has to still work afterwards. Without + // this a payload that was quietly dropped along with the rest of the + // parser state would pass. + emulator.writeInput("still alive\r\n".toByteArray()) + delay(100) + + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + val impl = emulator as TerminalEmulatorImpl + impl.processPendingUpdates() + val text = impl.snapshot.value.lines.joinToString("\n") { it.columnText } + assertTrue( + "terminal stopped echoing after malformed OSC payloads; got:\n$text", + text.contains("still alive"), + ) + } + + /** + * The report that led here came from a capability-probe burst, so the + * sequences arrive back to back rather than one per read. + */ + @Test + fun aBurstOfMalformedProbesDoesNotAbortTheProcess() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 24, initialCols = 80) + + var burst = ByteArray(0) + for ((_, payload) in malformedPayloads) { + burst += osc("0", payload) + burst += osc("1337", payload) + burst += osc("4", payload) + } + emulator.writeInput(burst) + delay(150) + + emulator.writeInput("survived\r\n".toByteArray()) + delay(100) + + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + val impl = emulator as TerminalEmulatorImpl + impl.processPendingUpdates() + val text = impl.snapshot.value.lines.joinToString("\n") { it.columnText } + assertTrue( + "terminal stopped echoing after a malformed probe burst; got:\n$text", + text.contains("survived"), + ) + } + + /** Well-formed multi-byte text must still survive the new decode path. */ + @Test + fun validMultiByteOscPayloadsStillWork() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 24, initialCols = 80) + + for (title in listOf("plain", "café", "日本語", "emoji 🎉", "mixed é日🎉")) { + emulator.writeInput(osc("0", title.toByteArray(Charsets.UTF_8))) + delay(10) + } + + emulator.writeInput("after unicode\r\n".toByteArray()) + delay(100) + + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + val impl = emulator as TerminalEmulatorImpl + impl.processPendingUpdates() + val text = impl.snapshot.value.lines.joinToString("\n") { it.columnText } + assertTrue( + "valid UTF-8 titles broke the terminal; got:\n$text", + text.contains("after unicode"), + ) + } + + /** + * The assertion that can actually fail on the host JVM. + * + * OSC 1337 `AddAnnotation=` reaches the fallback OSC handler — the one that + * builds the jstring — and stores its payload as segment metadata, so the + * decoded string is readable back out. Malformed bytes must arrive as U+FFFD + * with the surrounding valid text intact; the old path passed the raw bytes + * to NewStringUTF, which on this JVM yields mojibake instead. + */ + @Test + fun malformedBytesBecomeReplacementCharacters() = runBlocking { + val emulator = TerminalEmulatorFactory.create(initialRows = 24, initialCols = 80) + + // "ab" "cd" — valid text either side of one bad byte. + val payload = "AddAnnotation=ab".toByteArray() + + byteArrayOf(0x80.toByte()) + + "cd".toByteArray() + emulator.writeInput("hello".toByteArray()) + emulator.writeInput(osc("1337", payload)) + delay(150) + + InstrumentationRegistry.getInstrumentation().waitForIdleSync() + val impl = emulator as TerminalEmulatorImpl + impl.processPendingUpdates() + + val metadata = impl.snapshot.value.lines + .flatMap { it.semanticSegments } + .mapNotNull { it.metadata } + assertTrue( + "no annotation metadata was recorded at all; got $metadata", + metadata.isNotEmpty(), + ) + val annotation = metadata.first() + assertTrue( + "the bad byte should decode to U+FFFD, got: ${annotation.map { it.code.toString(16) }}", + annotation.contains('\uFFFD'), + ) + assertTrue( + "valid text either side of the bad byte should survive, got: $annotation", + annotation.contains("ab") && annotation.contains("cd"), + ) + } +}