Skip to content

Commit 78ad272

Browse files
awehttamclaude
andcommitted
fix(security): strip terminal escape sequences from message text on telnet/ssh read paths
Echomail/netmail bodies, kludge lines, subjects and author names were written to Telnet/SSH readers with no filtering of ANSI/VT control sequences. A crafted message (from any local poster, or any FidoNet uplink) could drive the reader's terminal: cursor/screen manipulation, display spoofing, and on permissive emulators OSC title/clipboard writes or answerback-query input injection. Tracked as GHSA-4225-c933-76f3. Add BinktermPHP\TerminalTextSanitizer: whitelist SGR colour sequences (ESC [ ... m) and TAB/CR/LF, remove every other escape sequence and C0/C1 control byte (cursor moves, erase/scroll, mode changes, OSC/DCS strings, charset designation, stray ESC). Applied at: - EchomailHandler / NetmailHandler message viewers (body + kludge lines) - MailUtils::quoteMessage() (reply/forward bodies, so the attack is not relayed onward through the composer) - TelnetUtils::formatMessageListEntry() and buildMessageHeaderBox() (list rows and header field values) - PacketBbs\PacketBbsTextRenderer, replacing its narrower escape strip Add unit tests and document the sanitizer contract and call sites in docs/TerminalServerDevGuide.md and docs/UPGRADING_1.10.5.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MdwzvTCrBAEf739qVccfmX
1 parent a509f41 commit 78ad272

9 files changed

Lines changed: 263 additions & 10 deletions

docs/TerminalServerDevGuide.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,24 @@ $statusLine = TelnetUtils::buildStatusBar($segments, $width);
290290

291291
If a widget genuinely lacks a capability needed by multiple features, extend it in `TelnetUtils` — do not work around it in a handler. When adding or extending a widget, update the table in `telnet/CLAUDE.md`.
292292

293+
### Sanitizing untrusted text for terminal display
294+
295+
Message bodies, kludge lines, subjects and author names can come from any local
296+
user or any upstream FTN node and are rendered close to verbatim by the read
297+
paths. Before such text is word-wrapped or written to the terminal it must pass
298+
through `BinktermPHP\TerminalTextSanitizer::sanitize()`, which keeps SGR colour
299+
sequences (`ESC [ … m`) and TAB/CR/LF while removing every other escape sequence
300+
and C0/C1 control byte (cursor/erase moves, OSC title/clipboard writes,
301+
DCS/answerback queries, etc.).
302+
303+
Current call sites: `EchomailHandler` / `NetmailHandler` message viewers
304+
(`message_text` + combined kludge lines), `MailUtils::quoteMessage()` (reply and
305+
forward bodies), `TelnetUtils::formatMessageListEntry()` and
306+
`TelnetUtils::buildMessageHeaderBox()` (list rows and header fields), and
307+
`PacketBbs\PacketBbsTextRenderer` (which then also drops the SGR codes, since
308+
radio links are plain text). Any new surface that renders remote message content
309+
must call the sanitizer too.
310+
293311
### Status Bar Discipline
294312

295313
The bottom status bar has limited width. Keep it to the **most-used primary actions only** — typically scroll, prev/next, reply, and quit. Every other key belongs exclusively in the Ctrl-K help overlay.

docs/UPGRADING_1.10.5.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ Make sure you have a current backup of your database and files before upgrading.
2020
- [Registration House Rules](#registration-house-rules)
2121
- [Full-Screen Editor Flicker](#full-screen-editor-flicker)
2222
- [CP437 Login ANSI Art](#cp437-login-ansi-art)
23+
- [Message Body Escape-Sequence Filtering](#message-body-escape-sequence-filtering)
2324
- [Door Games](#door-games)
2425
- [Door Player Backspace Handling](#door-player-backspace-handling)
2526
- [BBSDEV.DRP Drop File (Experimental)](#bbsdevdrp-drop-file-experimental)
@@ -69,6 +70,7 @@ Make sure you have a current backup of your database and files before upgrading.
6970
- **Registration house rules:** the terminal server's **Register new account** flow now shows the house rules in a paged box and requires the prospective user to type `YES` to accept them before any account details are collected. Declining aborts registration. Custom house rules from **Admin -> Appearance -> Content -> House Rules** are shown when set; otherwise the built-in default rule set is used. The browser registration page already linked to the same rules.
7071
- **Full-screen editor flicker:** the terminal server's full-screen message editor (used automatically when the terminal has 15 or more rows) no longer erases and repaints the entire screen after every keystroke. Typing within a line now updates only that line, cursor movement emits only a cursor move, and structural edits repaint just the text area — borders and the footer stay put. This removes the constant blue-background blink that was visible while composing, especially on larger terminals or higher-latency connections. Terminals with ANSI colour disabled keep the previous full-redraw behaviour. Fixes issue #432.
7172
- **CP437 login ANSI art:** the ANSI login screen (`ansi_prompt` display mode) now accepts `.ans` files saved in Code Page 437 by DOS / Synchronet tools. The high-byte box-drawing and block characters are converted to UTF-8 for display, and a trailing SAUCE / EOF record is stripped. Previously these bytes rendered as replacement characters, and the admin appearance editor could not load or save such art.
73+
- **Message body escape-sequence filtering (security fix, GHSA-4225-c933-76f3):** echomail and netmail bodies, kludge lines, subjects, and author names are now stripped of terminal control sequences before being shown to a Telnet or SSH reader. Previously a message containing raw ANSI/VT escape codes could move the reader's cursor, repaint or erase their screen, spoof displayed content, and — on terminal emulators that honour them — set the window title, write the clipboard, or inject input via an answerback query. Because echomail is FidoNet-federated, such a message could originate from any user on any connected node. ANSI colour (SGR) codes are preserved; cursor positioning, screen clears, and OSC/DCS sequences are removed, so genuine ANSI-art messages keep their colours but lose absolute cursor placement when read on a terminal.
7274

7375
### Door Games
7476

@@ -288,6 +290,34 @@ Those raw CP437 bytes are not valid UTF-8. When passed through template output t
288290

289291
`AppearanceConfig::getLoginScreenAnsi()` now truncates the content at the `0x1A` delimiter to drop any EOF / SAUCE block, and converts non-UTF-8 content from CP437 to UTF-8 with `iconv()` (falling back to `mb_convert_encoding()`), matching how shell art is already handled elsewhere. `AdminDaemonServer::getAppearanceConfig()` performs the same conversion before returning the JSON payload, so the **Admin -> Appearance** editor can load, edit, and save CP437 ANSI art without encoding errors.
290292

293+
### Message Body Escape-Sequence Filtering
294+
295+
This release fixes a stored terminal-escape-injection vulnerability, tracked as **GHSA-4225-c933-76f3** (severity: medium). It affects the Telnet and SSH terminal server; the browser interface was never exposed, because HTML output escapes these bytes.
296+
297+
#### The problem
298+
299+
A FidoNet message body is stored and later displayed to a terminal reader with very little transformation. When the reader opened an echomail or netmail message over Telnet or SSH, the terminal server passed the body through a word-wrapper (or the Markdown/StyleCodes renderer) and then a character-set conversion, and wrote the result straight to the socket. None of those steps removed ANSI/VT control sequences that were already in the body.
300+
301+
An ANSI/VT terminal interprets escape sequences in the byte stream as commands. A message body could therefore contain sequences that:
302+
303+
- move the cursor, scroll the screen, or clear regions of it, to garble or hide other content;
304+
- redraw parts of the screen to impersonate a system prompt or another user's message (display spoofing);
305+
- on emulators that honour them, set the terminal window title, write to the system clipboard (OSC 52), or issue a device-status / answerback query whose reply is injected back into the session as if the user had typed it.
306+
307+
Any account that can post a message could target any reader. Because echomail is federated across FidoNet, a crafted body could also arrive from a user on any connected uplink — the attacker did not need an account on your board. The subject line and author name shown in the message header and message list were exposed the same way. This is terminal manipulation on the reader's client, not code execution on the server.
308+
309+
#### The fix
310+
311+
A new filter, `BinktermPHP\TerminalTextSanitizer`, is applied to untrusted text on every terminal read path — the echomail and netmail message viewers (body and kludge lines), quoted and forwarded text placed in the composer, and the message-list rows and header fields. The same filter replaces the narrower escape strip that was already present on the MeshCore / PacketBBS radio renderer.
312+
313+
The filter keeps SGR (colour and text-style) sequences — `ESC [ … m` — and the TAB, CR, and LF whitespace controls. Everything else is removed: cursor movement, erase and scroll commands, mode changes, OSC and DCS strings, character-set designation, other escape sequences, and stray C0/C1 control bytes.
314+
315+
The visible effect for readers is that message colours are unchanged, but a message that relied on cursor positioning to draw ANSI art (as opposed to plain coloured text) will show that art without the positioning when read on a terminal. This path never rendered positioned art correctly in any case.
316+
317+
#### If you run a public terminal server
318+
319+
Upgrade promptly; there is no workaround short of disabling terminal access to messages. Filtering happens at display time, so it also covers messages that are already stored.
320+
291321
## Door Games
292322

293323
### Door Player Backspace Handling

src/PacketBbs/PacketBbsTextRenderer.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,11 @@ private function messageDate(string $date): string
318318
*/
319319
private function wrapBody(string $text): array
320320
{
321-
// Strip ANSI escape sequences
322-
$text = preg_replace('/\x1b\[[0-9;]*[mKHJABCDf]/', '', $text);
321+
// Strip terminal control sequences (radio links render plain text only).
322+
$text = \BinktermPHP\TerminalTextSanitizer::sanitize($text);
323+
// Radio display has no use for colour either — drop the SGR codes the
324+
// sanitizer preserves.
325+
$text = preg_replace('/\x1b\[[0-9;:]*m/', '', $text);
323326
$text = str_replace(["\r\n", "\r"], "\n", $text);
324327
$lines = explode("\n", $text);
325328
$output = [];

src/TerminalTextSanitizer.php

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php
2+
3+
namespace BinktermPHP;
4+
5+
/**
6+
* Sanitizes untrusted text (FTN message bodies, kludge lines, forwarded/quoted
7+
* content, subjects, author names) before it is rendered to an ANSI/VT terminal
8+
* over the Telnet, SSH, QWK or packet-BBS surfaces.
9+
*
10+
* A message body can arrive from any local user or any upstream FTN node and is
11+
* displayed more or less verbatim by the terminal read paths. Without filtering,
12+
* a body containing raw escape sequences can drive the reader's terminal:
13+
* cursor and screen manipulation, display spoofing, and — on emulators that
14+
* honour them — OSC title/clipboard writes or answerback/device-status queries
15+
* that reflect input back into the session.
16+
*
17+
* The policy here is a whitelist: SGR (Select Graphic Rendition) sequences
18+
* (`ESC [ ... m`) are kept so ANSI colour survives; every other escape
19+
* sequence and every C0/C1 control byte except TAB, CR and LF is removed.
20+
*/
21+
class TerminalTextSanitizer
22+
{
23+
/**
24+
* Strip terminal control sequences from untrusted text, keeping only SGR
25+
* colour/style codes and the TAB/CR/LF whitespace controls.
26+
*
27+
* The input is expected to be UTF-8 (the canonical storage form for message
28+
* text); charset conversion to CP437/ASCII happens downstream and does not
29+
* reintroduce an ESC introducer.
30+
*
31+
* @param string $text Raw untrusted text.
32+
* @return string Text safe to word-wrap and write to a terminal.
33+
*/
34+
public static function sanitize(string $text): string
35+
{
36+
if ($text === '') {
37+
return $text;
38+
}
39+
40+
// Split on well-formed SGR sequences, keeping them as captured
41+
// delimiters. Odd-indexed parts are the SGR sequences to preserve;
42+
// even-indexed parts are ordinary text that gets fully scrubbed.
43+
$parts = preg_split(
44+
'/(\x1b\[[0-9;:]*m)/',
45+
$text,
46+
-1,
47+
PREG_SPLIT_DELIM_CAPTURE
48+
);
49+
50+
if ($parts === false) {
51+
return self::scrub($text);
52+
}
53+
54+
$out = '';
55+
foreach ($parts as $i => $part) {
56+
$out .= ($i % 2 === 1) ? $part : self::scrub($part);
57+
}
58+
59+
return $out;
60+
}
61+
62+
/**
63+
* Remove every escape sequence and disallowed control byte from a fragment
64+
* that is known to contain no SGR sequences worth keeping.
65+
*/
66+
private static function scrub(string $text): string
67+
{
68+
if ($text === '') {
69+
return $text;
70+
}
71+
72+
// OSC (Operating System Command): ESC ] ... (BEL | ST). Window titles,
73+
// clipboard writes and answerback on permissive emulators.
74+
$text = preg_replace('/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\\\)?/', '', $text);
75+
76+
// DCS / SOS / PM / APC strings: ESC (P|X|^|_) ... ST.
77+
$text = preg_replace('/\x1b[PX^_][^\x1b]*(?:\x1b\\\\)?/', '', $text);
78+
79+
// Any CSI sequence (all non-SGR by construction, plus malformed or
80+
// unterminated ones): cursor movement, erase, scroll region, mode
81+
// changes, device-status queries.
82+
$text = preg_replace('/\x1b\[[0-9;:?<>=]*[ -\/]*[@-~]?/', '', $text);
83+
84+
// Character-set designation: ESC ( B , ESC ) 0 , ESC * A , ...
85+
$text = preg_replace('/\x1b[()*+\-.\/][0-9A-Za-z]/', '', $text);
86+
87+
// Any other two-byte escape (ESC c, ESC 7, ESC =, ...) and stray ESC.
88+
$text = preg_replace('/\x1b[\x20-\x7e]?/', '', $text);
89+
90+
// Remaining C0 control bytes except TAB (0x09), LF (0x0A), CR (0x0D),
91+
// plus DEL (0x7F).
92+
$text = preg_replace('/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/', '', $text);
93+
94+
// UTF-8-encoded C1 control range (U+0080–U+009F) — 0x9B is an alternate
95+
// CSI introducer on some terminals.
96+
$text = preg_replace('/\xc2[\x80-\x9f]/', '', $text);
97+
98+
return $text;
99+
}
100+
}

telnet/src/EchomailHandler.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,9 +540,9 @@ private function displaySearchMessage($conn, array &$state, string $session, arr
540540
$this->server->logAction($state['username'] ?? 'unknown', "Echomail search: read message #{$id} in {$area}");
541541

542542
$detail = TelnetUtils::apiRequest($this->apiBase, 'GET', '/api/messages/echomail/' . urlencode($area) . '/' . $id, null, $session);
543-
$body = $detail['data']['message_text'] ?? '';
543+
$body = \BinktermPHP\TerminalTextSanitizer::sanitize($detail['data']['message_text'] ?? '');
544544
$markupFormat = $detail['data']['markup_format'] ?? null;
545-
$rawKludges = ($detail['data']['kludge_lines'] ?? '') . "\n" . ($detail['data']['bottom_kludges'] ?? '');
545+
$rawKludges = \BinktermPHP\TerminalTextSanitizer::sanitize(($detail['data']['kludge_lines'] ?? '') . "\n" . ($detail['data']['bottom_kludges'] ?? ''));
546546
$kludgeLines = TerminalMarkupRenderer::extractKludgeLines($rawKludges);
547547
$kludgeLines = array_map(fn(string $line): string => $this->server->encodeForTerminal($line), $kludgeLines);
548548
$imageRefs = TerminalMarkupRenderer::extractImageRefs((string)($markupFormat ?? ''), $body);
@@ -1677,9 +1677,9 @@ private function displayMessage($conn, array &$state, string $session, string $a
16771677

16781678
$this->server->logAction($state['username'] ?? 'unknown', "Echomail: read message #{$id} in {$area}");
16791679
$detail = TelnetUtils::apiRequest($this->apiBase, 'GET', '/api/messages/echomail/' . urlencode($area) . '/' . $id, null, $session);
1680-
$body = $detail['data']['message_text'] ?? '';
1680+
$body = \BinktermPHP\TerminalTextSanitizer::sanitize($detail['data']['message_text'] ?? '');
16811681
$markupFormat = $detail['data']['markup_format'] ?? null;
1682-
$rawKludges = ($detail['data']['kludge_lines'] ?? '') . "\n" . ($detail['data']['bottom_kludges'] ?? '');
1682+
$rawKludges = \BinktermPHP\TerminalTextSanitizer::sanitize(($detail['data']['kludge_lines'] ?? '') . "\n" . ($detail['data']['bottom_kludges'] ?? ''));
16831683
$kludgeLines = TerminalMarkupRenderer::extractKludgeLines($rawKludges);
16841684
$kludgeLines = array_map(fn(string $line): string => $this->server->encodeForTerminal($line), $kludgeLines);
16851685
$imageRefs = TerminalMarkupRenderer::extractImageRefs((string)($markupFormat ?? ''), $body);

telnet/src/MailUtils.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,11 @@ public static function sendMessage(string $apiBase, string $session, array $payl
310310
*/
311311
public static function quoteMessage(string $body, string $author, ?array $state = null): string
312312
{
313+
// The original body is untrusted (any user or upstream FTN node). Strip
314+
// terminal control sequences before it is placed in the composer, both
315+
// so the editor renders safely and so the attack is not relayed onward.
316+
$body = \BinktermPHP\TerminalTextSanitizer::sanitize($body);
317+
313318
$lines = explode("\n", $body);
314319
$quoted = [];
315320
$quoted[] = '';

telnet/src/NetmailHandler.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,10 +633,10 @@ private function displayMessage($conn, array &$state, string $session, int $page
633633

634634
$this->server->logAction($state['username'] ?? 'unknown', "Netmail: read message #{$id}");
635635
$detail = TelnetUtils::apiRequest($this->apiBase, 'GET', '/api/messages/netmail/' . $id, null, $session);
636-
$body = $detail['data']['message_text'] ?? '';
636+
$body = \BinktermPHP\TerminalTextSanitizer::sanitize($detail['data']['message_text'] ?? '');
637637
$markupFormat = $detail['data']['markup_format'] ?? null;
638638
$attachments = $detail['data']['attachments'] ?? [];
639-
$rawKludges = ($detail['data']['kludge_lines'] ?? '') . "\n" . ($detail['data']['bottom_kludges'] ?? '');
639+
$rawKludges = \BinktermPHP\TerminalTextSanitizer::sanitize(($detail['data']['kludge_lines'] ?? '') . "\n" . ($detail['data']['bottom_kludges'] ?? ''));
640640
$kludgeLines = TerminalMarkupRenderer::extractKludgeLines($rawKludges);
641641
$kludgeLines = array_map(fn(string $line): string => $this->server->encodeForTerminal($line), $kludgeLines);
642642
$imageRefs = TerminalMarkupRenderer::extractImageRefs((string)($markupFormat ?? ''), $body);

telnet/src/TelnetUtils.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,8 +1473,8 @@ public static function renderMessageListScreen(
14731473
*/
14741474
public static function formatMessageListEntry(array $msg, int $num, bool $selected, int $cols, array &$state): string
14751475
{
1476-
$from = $msg['from_name'] ?? 'Unknown';
1477-
$subject = $msg['subject'] ?? '(no subject)';
1476+
$from = \BinktermPHP\TerminalTextSanitizer::sanitize($msg['from_name'] ?? 'Unknown');
1477+
$subject = \BinktermPHP\TerminalTextSanitizer::sanitize($msg['subject'] ?? '(no subject)');
14781478
$dateShort = self::formatUserDate($msg['date_written'] ?? '', $state, false);
14791479
$line = self::formatMessageListLine($num, $from, $subject, $dateShort, $cols);
14801480
if (empty($msg['is_read'])) {
@@ -2640,6 +2640,14 @@ public static function buildMessageHeaderBox(int $width, array $fields, string $
26402640
$tl = '+'; $tr = '+'; $bl = '+'; $br = '+'; $hz = '-'; $vt = '|';
26412641
}
26422642

2643+
// Field values may be untrusted (subject / author from a remote message);
2644+
// strip terminal control sequences before they land in the header box.
2645+
foreach ($fields as $i => $field) {
2646+
if (isset($field['value']) && is_string($field['value'])) {
2647+
$fields[$i]['value'] = \BinktermPHP\TerminalTextSanitizer::sanitize($field['value']);
2648+
}
2649+
}
2650+
26432651
// Inner content width: box width minus two corner/vertical chars and two space pads
26442652
$innerWidth = max(0, $width - 4);
26452653
$hFill = str_repeat($hz, max(0, $width - 2));

0 commit comments

Comments
 (0)