fix(osc): decode OSC payloads to UTF-16 instead of aborting on bad bytes

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<jstring> 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<jstring> payloadStr(
+        env,
+        env->NewString(reinterpret_cast<const jchar*>(payloadUtf16.data()),
+                       static_cast<jsize>(payloadUtf16.size())));
     if (!payloadStr.get()) {
         LOGE("Failed to create jstring for OSC payload");
         return 0;

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<char16_t>(cp));
+    } else {
+        cp -= 0x10000;
+        out.push_back(static_cast<char16_t>(0xD800 + (cp >> 10)));
+        out.push_back(static_cast<char16_t>(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<const uint8_t*>(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<size_t>(end - p) < static_cast<size_t>(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;
+}

lib/src/main/cpp/mutf8.h

@@ -17,11 +17,31 @@
 
 #include <cstdint>
 #include <cstdlib>
+#include <string>
 
 #ifndef CB_TERM_MUTF8_H
 #define CB_TERM_MUTF8_H
 
 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

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 ] <command> ; <payload> 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" <lone continuation byte> "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"),
+        )
+    }
+}