fix(nostr): neutralize bidi/zero-width spoofing in displayed untrusted text
Untrusted strings rendered in the payment/UI strip stripped only ASCII control chars, not Unicode bidi overrides/isolates or zero-width format chars. An attacker could embed a right-to-left override or invisible codepoint in a name, memo, or subject to visually reorder or hide the displayed identity/amount (Trojan-Source style spoof). Funds were never at risk — they always go to the correct npub — but the DISPLAY could lie. Add one shared classifier, nostr::sanitize::is_display_dangerous, that flags all Cc controls (C0/C1/DEL via char::is_control) plus the bidi marks/overrides/isolates (U+200E/200F, U+061C, U+202A-202E, U+2066-2069) and zero-width/BOM chars (U+200B-200D, U+FEFF). Route every untrusted display surface through it: - sanitize_note (DM subject/note): dangerous -> space, was control-only. - validate_memo (pay-URI memo): now filters by char after decode so the multibyte bidi codepoints a byte filter missed are dropped. - escape_for_display (authorize/login prompts): delegates to the shared classifier, completing TODO(audit M5). - nip05::name_by_pubkey (authority @name): sanitized + capped at 64 chars before it becomes a rendered contact handle. Only invisible FORMAT/CONTROL codepoints are neutralized; legitimate letters of every script (Latin/accents, Arabic, Hebrew, CJK, Hangul) and emoji pass through unchanged — proven by tests.
This commit is contained in:
+5
-12
@@ -531,20 +531,13 @@ pub fn content_preview(content: &str) -> (String, usize) {
|
||||
/// passes through unchanged. Used on ALL requester-controlled strings (content,
|
||||
/// tag values, titles) before they reach a label.
|
||||
pub fn escape_for_display(s: &str) -> String {
|
||||
// TODO(audit M5): widen escaping to category-based (all C0/C1, bidi controls)
|
||||
// rather than the current explicit list.
|
||||
// Category-based, shared with every other untrusted-string surface (completes
|
||||
// the former TODO(audit M5)): all Cc controls (C0/C1/DEL), the bidi
|
||||
// overrides/isolates/marks, and the zero-width/BOM format chars.
|
||||
let mut out = String::with_capacity(s.len());
|
||||
for c in s.chars() {
|
||||
let code = c as u32;
|
||||
let dangerous = code < 0x20
|
||||
|| code == 0x7f
|
||||
|| c == '\u{200E}' // LEFT-TO-RIGHT MARK
|
||||
|| c == '\u{200F}' // RIGHT-TO-LEFT MARK
|
||||
|| c == '\u{061C}' // ARABIC LETTER MARK
|
||||
|| (0x202A..=0x202E).contains(&code) // LRE, RLE, PDF, LRO, RLO
|
||||
|| (0x2066..=0x2069).contains(&code); // LRI, RLI, FSI, PDI
|
||||
if dangerous {
|
||||
out.push_str(&format!("\\u{{{code:04X}}}"));
|
||||
if crate::nostr::sanitize::is_display_dangerous(c) {
|
||||
out.push_str(&format!("\\u{{{:04X}}}", c as u32));
|
||||
} else {
|
||||
out.push(c);
|
||||
}
|
||||
|
||||
@@ -49,6 +49,7 @@ pub use client::{HeldIdentityKeys, NostrProfile, NostrService, TransportStatus,
|
||||
|
||||
pub mod avatar;
|
||||
pub mod nip05;
|
||||
pub mod sanitize;
|
||||
|
||||
pub mod authuri;
|
||||
pub mod loginuri;
|
||||
|
||||
+11
-4
@@ -25,6 +25,11 @@ use crate::nostr::relays::HOME_NIP05_DOMAIN;
|
||||
use crate::tor;
|
||||
use parking_lot::RwLock;
|
||||
|
||||
/// Hard cap on a name-authority `@name` before it is shown as a contact handle.
|
||||
/// A display name has no business being hundreds of characters; this bounds a
|
||||
/// hostile/compromised authority that returns a wall of text.
|
||||
const MAX_NAME_CHARS: usize = 64;
|
||||
|
||||
/// The active name-authority "home" domain, mirrored here from the wallet config
|
||||
/// once per frame so resolution + display (some on worker threads) can read it
|
||||
/// without threading the config through every call site. `None` = the default
|
||||
@@ -122,10 +127,12 @@ pub async fn name_by_pubkey(domain: &str, pubkey_hex: &str) -> Option<String> {
|
||||
);
|
||||
let body = tor::http_request("GET", url, None, vec![]).await?;
|
||||
let doc: Value = serde_json::from_str(&body).ok()?;
|
||||
doc.get("name")
|
||||
.and_then(|v| v.as_str())
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(|s| s.to_string())
|
||||
let raw = doc.get("name").and_then(|v| v.as_str())?;
|
||||
// The authority is untrusted display data: strip any control/bidi/zero-width
|
||||
// codepoints and cap the length before this name becomes a contact handle
|
||||
// rendered as the payer/peer identity. A hostile or compromised authority
|
||||
// must not be able to spoof or reorder the displayed name.
|
||||
crate::nostr::sanitize::sanitize_name(raw, MAX_NAME_CHARS)
|
||||
}
|
||||
|
||||
/// Verify that a pubkey matches its claimed NIP-05 identifier.
|
||||
|
||||
+15
-10
@@ -242,19 +242,24 @@ fn validate_amount(raw: &str) -> Option<String> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Validate a `memo` value: percent-decode, strip ASCII control chars and
|
||||
/// newlines (untrusted free text — display / tx-message only, never a path or
|
||||
/// route), then hard-cap at [`MAX_MEMO_BYTES`] on a UTF-8 boundary. Empty →
|
||||
/// `None`.
|
||||
/// Validate a `memo` value: percent-decode, then drop every display-dangerous
|
||||
/// codepoint — ASCII control chars/newlines AND the bidi-override / isolate /
|
||||
/// zero-width format chars (see
|
||||
/// [`crate::nostr::sanitize::is_display_dangerous`]) — because a memo is
|
||||
/// untrusted free text rendered in the payment strip (display / tx-message
|
||||
/// only, never a path or route). Then hard-cap at [`MAX_MEMO_BYTES`] on a UTF-8
|
||||
/// boundary. Empty → `None`. Filtering by CHAR after decoding (not by byte) is
|
||||
/// what catches the multibyte bidi codepoints a byte filter would miss.
|
||||
fn validate_memo(raw: &str) -> Option<String> {
|
||||
let decoded = percent_decode(raw);
|
||||
// Drop ASCII control bytes (< 0x20, covering NUL / newline / tab) and DEL.
|
||||
let cleaned: Vec<u8> = decoded
|
||||
.into_iter()
|
||||
.filter(|&b| b >= 0x20 && b != 0x7f)
|
||||
let text = String::from_utf8_lossy(&decoded);
|
||||
// Drop control chars (NUL / newline / tab / DEL) and bidi/zero-width format
|
||||
// chars — leaving legitimate letters of every script intact.
|
||||
let cleaned: String = text
|
||||
.chars()
|
||||
.filter(|c| !crate::nostr::sanitize::is_display_dangerous(*c))
|
||||
.collect();
|
||||
let text = String::from_utf8_lossy(&cleaned).into_owned();
|
||||
let text = truncate_on_char_boundary(text, MAX_MEMO_BYTES);
|
||||
let text = truncate_on_char_boundary(cleaned, MAX_MEMO_BYTES);
|
||||
let text = text.trim().to_string();
|
||||
if text.is_empty() { None } else { Some(text) }
|
||||
}
|
||||
|
||||
+12
-3
@@ -50,12 +50,21 @@ static SLATEPACK_RE: LazyLock<Regex> = LazyLock::new(|| {
|
||||
Regex::new(r"BEGINSLATEPACK\.[\s\S]*?ENDSLATEPACK\.").expect("slatepack regex")
|
||||
});
|
||||
|
||||
/// Sanitize a user note: strip control characters, collapse whitespace,
|
||||
/// trim and cap the length. Returns `None` when nothing readable remains.
|
||||
/// Sanitize a user note: neutralize control AND bidi/zero-width format
|
||||
/// characters (see [`crate::nostr::sanitize::is_display_dangerous`]) to spaces,
|
||||
/// collapse whitespace, trim and cap the length. Returns `None` when nothing
|
||||
/// readable remains. Mapping to a space (rather than dropping) preserves word
|
||||
/// boundaries and keeps a hidden joiner from silently fusing two tokens.
|
||||
pub fn sanitize_note(raw: &str) -> Option<String> {
|
||||
let cleaned: String = raw
|
||||
.chars()
|
||||
.map(|c| if c.is_control() { ' ' } else { c })
|
||||
.map(|c| {
|
||||
if crate::nostr::sanitize::is_display_dangerous(c) {
|
||||
' '
|
||||
} else {
|
||||
c
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let collapsed = cleaned.split_whitespace().collect::<Vec<_>>().join(" ");
|
||||
let trimmed = collapsed.trim();
|
||||
|
||||
@@ -0,0 +1,186 @@
|
||||
// Copyright 2026 The Goblin Developers
|
||||
//
|
||||
// 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.
|
||||
|
||||
//! Display sanitization shared by every untrusted-string surface.
|
||||
//!
|
||||
//! A single classifier, [`is_display_dangerous`], decides whether a codepoint
|
||||
//! is unsafe to render raw in the payment/UI strip. It is the one source of
|
||||
//! truth behind the DM-note sanitizer, the memo/URI sanitizer, the
|
||||
//! name-authority `@name` cap, and the authorize/login prompt escaper. This
|
||||
//! completes `TODO(audit M5)`: the set is Unicode-category driven (Cc controls
|
||||
//! via [`char::is_control`], plus the explicit bidi / zero-width format
|
||||
//! codepoints) rather than an ad-hoc per-caller list.
|
||||
//!
|
||||
//! Threat: a right-to-left override, isolate, or invisible/zero-width codepoint
|
||||
//! embedded in an attacker-controlled name / memo / subject can visually
|
||||
//! reorder or hide the surrounding UI text so the DISPLAY lies about identity
|
||||
//! or amount (a "Trojan Source" style spoof). Funds are unaffected — they
|
||||
//! always go to the correct npub — but the rendered strip must not be
|
||||
//! forgeable.
|
||||
//!
|
||||
//! Non-goal: mangling legitimate non-Latin text. Only the invisible
|
||||
//! FORMAT/CONTROL codepoints are neutralized; the visible LETTERS of every
|
||||
//! script — Latin (incl. accents), Arabic, Hebrew, CJK, Hangul — and
|
||||
//! standalone emoji pass through untouched.
|
||||
|
||||
/// Whether `c` must never reach a label raw.
|
||||
///
|
||||
/// Neutralized set:
|
||||
/// - **Cc controls** — C0 `U+0000..=U+001F`, DEL `U+007F`, and C1
|
||||
/// `U+0080..=U+009F`. Matched by [`char::is_control`], which is exactly the
|
||||
/// `Cc` general category.
|
||||
/// - **Bidi controls / overrides / isolates** (the classic Trojan-Source
|
||||
/// vector): `U+202A..=U+202E` (LRE, RLE, PDF, LRO, RLO),
|
||||
/// `U+2066..=U+2069` (LRI, RLI, FSI, PDI), `U+200E`/`U+200F` (LRM/RLM),
|
||||
/// and `U+061C` (ARABIC LETTER MARK).
|
||||
/// - **Zero-width / invisible joiners and the BOM**: `U+200B` (ZWSP),
|
||||
/// `U+200C` (ZWNJ), `U+200D` (ZWJ), `U+FEFF` (ZWNBSP / BOM).
|
||||
///
|
||||
/// Deliberately NOT flagged: ordinary letters of any script and standalone
|
||||
/// emoji. Arabic and Hebrew LETTERS render right-to-left on their own and are
|
||||
/// perfectly safe — it is only the explicit override/isolate FORMAT codepoints
|
||||
/// that are dangerous.
|
||||
pub fn is_display_dangerous(c: char) -> bool {
|
||||
if c.is_control() {
|
||||
// Cc: C0, DEL, and the C1 block.
|
||||
return true;
|
||||
}
|
||||
matches!(
|
||||
c as u32,
|
||||
0x200E | 0x200F // LRM, RLM
|
||||
| 0x061C // ARABIC LETTER MARK
|
||||
| 0x202A..=0x202E // LRE, RLE, PDF, LRO, RLO
|
||||
| 0x2066..=0x2069 // LRI, RLI, FSI, PDI
|
||||
| 0x200B..=0x200D // ZWSP, ZWNJ, ZWJ
|
||||
| 0xFEFF // ZWNBSP / BOM
|
||||
)
|
||||
}
|
||||
|
||||
/// Sanitize an attacker-controlled display name (e.g. the `@name` an authority
|
||||
/// reports for a pubkey): drop every [`is_display_dangerous`] codepoint, trim
|
||||
/// surrounding whitespace, and hard-cap at `max_chars` characters. Returns
|
||||
/// `None` when nothing legible remains — a name that was empty, or was made
|
||||
/// only of control/bidi/zero-width chars, is not a usable handle.
|
||||
///
|
||||
/// A display name has no legitimate need for hundreds of characters, so the cap
|
||||
/// also bounds a hostile authority that returns a wall of text.
|
||||
pub fn sanitize_name(raw: &str, max_chars: usize) -> Option<String> {
|
||||
let cleaned: String = raw.chars().filter(|c| !is_display_dangerous(*c)).collect();
|
||||
let trimmed = cleaned.trim();
|
||||
if trimmed.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(trimmed.chars().take(max_chars).collect())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The full set of format/control codepoints the classifier must flag.
|
||||
const DANGEROUS: &[char] = &[
|
||||
'\u{0000}', '\u{0007}', '\u{001B}', '\u{007F}', // C0 + DEL
|
||||
'\u{0085}', '\u{009F}', // C1
|
||||
'\u{200E}', '\u{200F}', '\u{061C}', // marks
|
||||
'\u{202A}', '\u{202B}', '\u{202C}', '\u{202D}', '\u{202E}', // bidi overrides
|
||||
'\u{2066}', '\u{2067}', '\u{2068}', '\u{2069}', // bidi isolates
|
||||
'\u{200B}', '\u{200C}', '\u{200D}', // zero-width
|
||||
'\u{FEFF}', // BOM
|
||||
];
|
||||
|
||||
/// Legitimate, visible text from the locales Goblin ships — must ALL pass.
|
||||
const SAFE: &[&str] = &[
|
||||
"café", // accented Latin
|
||||
"Grüße", // German umlaut + ß
|
||||
"日本語", // Japanese / CJK
|
||||
"中文名字", // Chinese
|
||||
"한국어", // Korean Hangul
|
||||
"مرحبا", // Arabic letters (naturally RTL — safe)
|
||||
"שלום", // Hebrew letters (naturally RTL — safe)
|
||||
"Ñoño", // Spanish
|
||||
"🎉", // emoji
|
||||
"🍺 grin", // emoji + ASCII
|
||||
];
|
||||
|
||||
#[test]
|
||||
fn flags_every_dangerous_codepoint() {
|
||||
for &c in DANGEROUS {
|
||||
assert!(
|
||||
is_display_dangerous(c),
|
||||
"{:?} (U+{:04X}) must be flagged",
|
||||
c,
|
||||
c as u32
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn passes_every_legitimate_letter() {
|
||||
for s in SAFE {
|
||||
for c in s.chars() {
|
||||
assert!(
|
||||
!is_display_dangerous(c),
|
||||
"{:?} (U+{:04X}) in {:?} must NOT be flagged",
|
||||
c,
|
||||
c as u32,
|
||||
s
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_name_neutralizes_bidi_and_zero_width() {
|
||||
// A right-to-left override that would visually reverse "1cnp" → "npc1".
|
||||
assert_eq!(
|
||||
sanitize_name("Alice\u{202E}eve", 64),
|
||||
Some("Aliceeve".to_string())
|
||||
);
|
||||
// Zero-width joiner splicing two look-alike halves.
|
||||
assert_eq!(
|
||||
sanitize_name("go\u{200D}blin", 64),
|
||||
Some("goblin".to_string())
|
||||
);
|
||||
// BOM + control noise.
|
||||
assert_eq!(
|
||||
sanitize_name("\u{FEFF}bob\u{0000}", 64),
|
||||
Some("bob".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_name_preserves_non_latin_scripts() {
|
||||
for s in SAFE {
|
||||
assert_eq!(
|
||||
sanitize_name(s, 64).as_deref(),
|
||||
Some(s.trim()),
|
||||
"{s:?} must survive sanitization unchanged"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_name_caps_length() {
|
||||
let long = "本".repeat(200); // 200 CJK chars, none dangerous
|
||||
let out = sanitize_name(&long, 64).expect("non-empty");
|
||||
assert_eq!(out.chars().count(), 64);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_name_rejects_all_noise() {
|
||||
assert_eq!(sanitize_name("\u{202E}\u{200B}\u{0000}", 64), None);
|
||||
assert_eq!(sanitize_name(" ", 64), None);
|
||||
assert_eq!(sanitize_name("", 64), None);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user