Close the system-message spoofing hole in the action parser - #1662
Close the system-message spoofing hole in the action parser#1662jackjackbits wants to merge 3 commits into
Conversation
processActionMessage rewrote the sender to "system" — the formatter's trusted styling — for ANY `* … *` content containing 🫂, 🐟, or the substring "took a screenshot". A hostile peer could render arbitrary text as a system-authored line: `* SECURITY: your session key expired, re-verify at evil.example — bob took a screenshot *`. Only the exact locally-generatable action shapes qualify now, and the actor slot must equal the actual wire sender: a peer can hug, slap, or screenshot only as themselves, and the target slot is bounded, single-line, and nickname-shaped. Same templates iOS and Android emit, so legit actions render unchanged; anything else falls through as an ordinary peer message under the sender's real name. Pinned by tests covering the legit shapes, the original spoof payload, actor-mismatch, and free-text-in-target-slot cases. No new strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a05c67252f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard inner.hasPrefix(prefix), inner.hasSuffix(suffix), | ||
| inner.count >= prefix.count + suffix.count + 1 else { return false } | ||
| let target = inner.dropFirst(prefix.count).dropLast(suffix.count) | ||
| return !target.isEmpty && target.count <= 64 && !target.contains("\n") |
There was a problem hiding this comment.
For an inbound public message such as * 🫂 bob hugs SECURITY: session expired—visit evil.example *, this check accepts the entire short, single-line payload as the target even though it is not a resolved nickname, after which processActionMessage rewrites the sender to system. Because CommandProcessor.handleEmote only generates actions for nicknames resolved by getPeerIDForNickname, length and newline checks do not establish an exact locally generated shape; the system-message spoofing path therefore remains available with an action preamble.
Useful? React with 👍 / 👎.
| let isActionMessage = | ||
| Self.matchesActionTemplate(inner, prefix: "🫂 \(sender) hugs ", suffix: "") | ||
| || Self.matchesActionTemplate(inner, prefix: "🐟 \(sender) slaps ", suffix: " around a bit with a large trout") | ||
| || inner == "\(sender) took a screenshot" |
There was a problem hiding this comment.
In location channels, CommandProcessor.handleEmote embeds the unsuffixed context.nickname in /hug and /slap content, while NostrInboundPipeline sets message.sender through displayNameForNostrPubkey, which appends #<pubkey suffix>. Thus an ordinary generated action such as * 🫂 bob hugs alice#1234 * arrives with a sender like bob#ab12 and fails these actor-anchored templates, regressing every received location-chat action to ordinary peer text instead of system-action rendering.
Useful? React with 👍 / 👎.
- Codex P1: bounded-single-line was not "exact locally generated
shape" — a self-attributed action could smuggle a preamble into the
target slot ("… hugs SECURITY: reset your keys at evil…"). The
target must now be a single name token ("you" or a whitespace-free
≤32-char nickname, optionally #abcd-suffixed); free text with spaces
degrades to a plain message.
- Codex P1: location-channel senders arrive suffixed (bob#ab12) while
handleEmote embeds the unsuffixed nickname, so the actor match
regressed every received geohash action to plain text. The actor is
now compared by base name (splitSuffix), restoring legit rendering.
Tests cover the suffixed-sender case, the suffixed target, and the
preamble spoof.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both Codex findings addressed:
|
Chessing234
left a comment
Chessing234
left a comment
There was a problem hiding this comment.
the actor-anchoring is the right shape and the template match kills the preamble case. but the target slot still admits the payload your own example is built around.
isNameToken accepts any whitespace-free token up to 32 chars. https://evil.tld is 16 chars with no whitespace, so:
* 🫂 <attacker's own nick> hugs https://evil.tld *
passes matchesActionTemplate, the actor equals the wire sender, and it renders as system. and the content body isn't skipped for system senders — formatMessage's sender != "system" guard only skips the <@name> prefix, the match loop below still runs — so hasURLHint fires on :// and the token comes out blue, underlined, with matchStyle.link set. attacker-chosen tappable link under trusted styling, one packet, no preamble needed.
www.evil.tld works the same way via the www. hint.
the bound that would hold is the nickname charset rather than "no spaces": handleEmote only ever puts a resolved nickname or you there, and nicknames don't contain : or /. rejecting those two characters closes it; matching the actual #abcd suffix shape closes it properly.
2 participants
Tier-1 batch 4/6. processActionMessage rewrote the sender to "system" — the formatter's trusted styling — for ANY
* … *content containing 🫂, 🐟, or the substring "took a screenshot", so a hostile peer could render arbitrary text as a system-authored line (* SECURITY: your session key expired, re-verify at evil.example — bob took a screenshot *). Only the exact locally-generatable action shapes qualify now, and the actor slot must equal the actual wire sender — a peer can hug/slap/screenshot only as themselves; the target slot is bounded, single-line, nickname-shaped. Same templates iOS and Android emit, so legit actions render unchanged; everything else falls through as an ordinary peer message under the sender's real name. Pinned by tests incl. the original spoof payload and actor-mismatch cases. No new strings.🤖 Generated with Claude Code