Skip to content

parseGroupAddress: a localhost prefix match downgrades an attacker-chosen host to plaintext ws:// #78

Description

@usekaneo

From the CON-43 security audit (2026-09-14). Severity medium, reachable in the shipped app. Found while verifying the audit's relay-trust finding — this one is a plain bug, not a design question.

src/nostr/group-address.ts:21-22:

const local = host.startsWith('localhost') || host.startsWith('127.0.0.1')
return { host, id, relayUrl: `${local ? 'ws' : 'wss'}://${host}` }

startsWith is a prefix test, not an equality test. Verified with node:

'localhost.evil.example'.startsWith('localhost')  // true  -> ws://localhost.evil.example

So any host that merely begins with localhost or 127.0.0.1 — localhost.attacker.tld, 127.0.0.1.attacker.tld, localhostile.example — is treated as local and gets an unencrypted ws:// connection. A domain like that is trivially registrable; the prefix is not a property an attacker has to defeat, it is one they pick.

Why it matters

The host comes out of the route (/s/<host>'<group>), so it is attacker-choosable via a link. src/ui/layout/AppShell.tsx:37-39 feeds the parsed relayUrl straight into useRelay() → client.want(), which opens the connection. The transport then carries a NIP-42 AUTH event that NostrClient signs automatically (src/nostr/client.ts:172: this.pool.automaticallyAuth = (url) => this.signAuth(url)), i.e. the signed-in user's pubkey and a valid signature travel in the clear, over a connection any network observer on the path can read and modify.

The broader question — whether the app should connect to a relay named in a link at all — is deliberately not this ticket; see the sibling ticket. This one is the narrow bug: whatever the answer there, startsWith is the wrong test and the downgrade to ws:// is not intended behaviour.

Task

  • Match the local case exactly instead of by prefix. localhost and 127.0.0.1 with an optional :<port> — nothing else. A small helper with the rule in one place, not a second inline condition.
  • Consider whether [::1] belongs in the local set; today it does not and gets wss://, which fails on a dev relay. Decide, do not leave it implicit.
  • While in this function: parseGroupAddress does no validation of the host's shape at all beyond "there is an apostrophe". A host containing /, ?, #, @ or whitespace produces a relayUrl that is not the URL it looks like — evil.example/#@real-relay.example and friends. Reject anything that is not a plain host[:port].

Acceptance

  • localhost.evil.example, 127.0.0.1.evil.example and similar resolve to wss://, not ws://.
  • localhost, localhost:8080, 127.0.0.1:8080 still resolve to ws:// — the dev setup from scripts/dev-relay-up.sh keeps working unchanged.
  • A host that is not a plain host[:port] is rejected (null), so the route falls through to not-found rather than connecting somewhere unintended.
  • Unit tests in src/nostr/group-address.test.ts cover each of those three cases explicitly, including the prefix-attack strings — a test that only checks localhost would have passed against the buggy code too.
  • npm run typecheck && npm run lint && npm test && npm run build pass.

Affected: src/nostr/group-address.ts

Source: CON-43, extending finding A4.


Task: oozaxfd8qw29r3tk11k4ikn8

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions