Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 49 additions & 3 deletions lib/core/models/relay_list_event.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import 'package:dart_nostr/dart_nostr.dart';
import 'package:mostro_mobile/core/test_environment.dart';
import 'package:mostro_mobile/services/logger_service.dart';
import 'package:mostro_mobile/shared/utils/nostr_utils.dart';

/// Matches every trailing slash so `wss://relay.example//` normalizes the same
/// way as `wss://relay.example/`.
Expand All @@ -17,19 +20,54 @@ class RelayListEvent {
required this.authorPubkey,
});

/// Upper bound on the relays one event may contribute.
///
/// A node's NIP-65 list is a handful of entries. The number is a sanity
/// bound on an attacker-controlled list, not a policy: without one, a single
/// event can name as many relays as it likes and every one of them joins the
/// active set and receives every subscription.
static const int maxRelays = 50;

/// Parses a kind 10002 Nostr event into a RelayListEvent.
/// Returns null if the event is not a valid kind 10002 event.
///
/// Returns null unless the event is a kind-10002 event that the author it
/// claims actually signed. This is the only way a [RelayListEvent] is
/// produced, so the check lives here rather than at each call site: what the
/// list names becomes the app's relay set, which is the position every other
/// relay-sourced attack is launched from. The author field on its own is a
/// relay's claim about an event it chose to hand over.
///
/// Verifying here also makes [publishedAt] mean something: the freshness
/// comparison downstream is against `created_at`, which is covered by the
/// signature and the recomputed id, so a relay cannot backdate or
/// future-date a list to control which one wins.
static RelayListEvent? fromEvent(NostrEvent event) {
if (event.kind != 10002) return null;

if (!NostrUtils.isValidEventSignature(event)) {
logger.w(
'Rejecting kind-10002 relay list claiming to be from ${event.pubkey}: '
'signature verification failed',
);
return null;
}

// Extract relay URLs from 'r' tags
final relays = event.tags
var relays = event.tags
?.where((tag) => tag.isNotEmpty && tag[0] == 'r')
.where((tag) => tag.length >= 2)
.map((tag) => tag[1])
.where((url) => url.isNotEmpty)
.toList() ?? <String>[];

if (relays.length > maxRelays) {
logger.w(
'Relay list from ${event.pubkey} names ${relays.length} relays; '
'keeping the first $maxRelays',
);
relays = relays.sublist(0, maxRelays);
}

Comment on lines +63 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the relay cap after filtering and deduplication

When a signed relay list contains more than 50 raw r tags, this truncates the raw list before validRelays removes malformed, insecure, or duplicate entries. For example, 50 duplicate or invalid tags followed by the node's usable relays produces an empty or one-relay active set and discards all later valid relays, potentially leaving the app unable to reach the Mostro instance. Normalize, validate, and deduplicate first, then cap the relays that can actually be contributed.

Useful? React with 👍 / 👎.

// Handle different possible types for createdAt
DateTime publishedAt;
if (event.createdAt is DateTime) {
Expand All @@ -49,9 +87,17 @@ class RelayListEvent {

/// Validates that all relay URLs are properly formatted WebSocket URLs
/// Also normalizes URLs by removing trailing slashes to prevent duplicates
///
/// Cleartext `ws://` is refused on the same terms as the manual-entry path:
/// only inside the Mortsom test environment, where the relay is a local
/// process on a private address. Accepting it here was a way around that
/// rule — a node's list could downgrade the app's transport to plaintext
/// without the user ever typing a URL.
List<String> get validRelays {
return relays
.where((url) => url.startsWith('wss://') || url.startsWith('ws://'))
.where((url) =>
url.startsWith('wss://') ||
(TestEnvironment.allowInsecureRelays && url.startsWith('ws://')))
.map((url) => url.trim())
.map((url) => url.replaceAll(_trailingSlashes, ''))
.toList();
Expand Down
5 changes: 4 additions & 1 deletion lib/data/models/nostr_event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ extension NostrEventExtensions on NostrEvent {
DateTime get expirationDate => _getTimeStamp(_getTagValue('expiration')!);
String? get expiresAt => _getTagValue('expires_at');
String? get platform => _getTagValue('y');
String get type => _getTagValue('z')!;
/// The NIP-69 `z` tag. Nullable on purpose: this is read off relay-supplied
/// events before anything about them has been established, so a missing tag
/// has to be something the intake can reject rather than crash on.
String? get type => _getTagValue('z');

String? _getTagValue(String key) {
final tag = tags?.firstWhere((t) => t[0] == key, orElse: () => []);
Expand Down
35 changes: 33 additions & 2 deletions lib/data/repositories/open_orders_repository.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,39 @@ class OpenOrdersRepository implements OrderRepository<NostrEvent> {
);

_subscription = _nostrService.subscribeToEvents(request).listen((event) {
if (event.type == 'order') {
_events[event.orderId!] = event;
if (event.kind == orderEventKind &&
event.pubkey == _settings.mostroPublicKey) {
// The filter above states what was asked for, never what arrives. The
// pinned dart_nostr fork parses relay EVENT frames straight into a
// NostrEvent: it neither verifies the signature nor matches the frame
// against the subscription's filter. So `authors` constrains the
// request, and the author field on the way back is the relay's claim —
// the signature is what turns it into the node's.
//
// The order book is what this feeds: amounts, premiums and the maker
// rating a user reads before deciding to trade.
if (!NostrUtils.isValidEventSignature(event)) {
logger.w(
'Rejecting kind-$orderEventKind order event claiming to be from '
'${event.pubkey}: signature verification failed',
);
return;
}

// Both tags are read off the event itself — one to key the cache, one
// to classify it — so neither can be taken for granted, and a missing
// one used to throw inside this callback rather than be rejected.
final orderId = event.orderId;
if (orderId == null || event.type != 'order') {
logger.w(
'Ignoring kind-$orderEventKind event ${event.id} from '
'${event.pubkey}: expected a d tag and z=order, got d=$orderId '
'and z=${event.type}',
);
return;
}

_events[orderId] = event;
_eventStreamController.add(_events.values.toList());
} else if (event.kind == infoEventKind &&
event.pubkey == _settings.mostroPublicKey) {
Expand Down
77 changes: 41 additions & 36 deletions lib/services/nostr_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -496,52 +496,57 @@ class NostrService {
}
}

/// Fetches events from specific relays temporarily
/// Fetches events from [relays] on a connection of their own, leaving the
/// app's relay pool untouched.
///
/// These URLs come from outside the user's configuration: a `mostro:` deep
/// link supplies them, so whoever gets a link tapped picks them. They used
/// to be merged into `settings.relays` and connected through [updateSettings],
/// with a restore afterwards that could not work — it passed the `settings`
/// getter, already replaced by then, so the relay comparison was the
/// temporary list against itself and skipped as unchanged.
///
/// Restoring the settings correctly would not have been enough either.
/// dart_nostr's `init` is additive and the fork exposes no per-relay
/// disconnect, so once the app connects to a relay it stays connected for
/// the process lifetime — receiving every subscription filter the app
/// issues, every trade pubkey among them, and able to feed the app's
/// intakes. Nothing short of not connecting it to the shared pool works.
///
/// The scoped instance is closed when the fetch ends, on every path. Same
/// approach the relay connectivity test uses.
Future<List<NostrEvent>> _fetchFromSpecificRelays(
NostrFilter filter,
List<String> relays,
) async {
try {
// Store current relays
final originalRelays = List<String>.from(settings.relays);

// Temporarily add specific relays if not already present
final allRelays = <String>{...originalRelays, ...relays}.toList();

if (!ListEquality().equals(originalRelays, allRelays)) {
logger.i('Temporarily connecting to additional relays: $relays');

// Update settings with additional relays
final tempSettings = Settings(
relays: allRelays,
mostroPublicKey: settings.mostroPublicKey,
fullPrivacyMode: settings.fullPrivacyMode,
defaultFiatCode: settings.defaultFiatCode,
selectedLanguage: settings.selectedLanguage,
);

await updateSettings(tempSettings);

// Fetch the events
final events = await fetchEvents(filter);
logger.i('Fetching from link-supplied relays on a scoped connection: '
'$relays');

// Restore original relays
await updateSettings(settings);
final scoped = Nostr();
try {
await scoped.services.relays.init(
relaysUrl: relays,
connectionTimeout: Config.relayConnectionTimeout,
shouldReconnectToRelayOnNotice: false,
retryOnClose: false,
retryOnError: false,
);

return events;
} else {
// No new relays to add, use normal fetch
return await fetchEvents(filter);
}
return await scoped.services.relays.startEventsSubscriptionAsync(
request: NostrRequest(filters: [filter]),
timeout: Config.nostrOperationTimeout,
);
} catch (e) {
logger.e('Error fetching from specific relays: $e');
// Ensure we restore original settings even on error
rethrow;
} finally {
try {
await updateSettings(settings);
} catch (restoreError) {
logger.e('Failed to restore original relay settings: $restoreError');
await scoped.services.relays.disconnectFromRelays();
} catch (e) {
// The fetch already returned; a connection left open here would still
// be on the scoped instance, never on the app's pool.
logger.w('Failed to close the scoped relay connection: $e');
}
rethrow;
}
}
}
Loading