This commit introduces a verification step at the relay connection
level, to help ensure notes get validated at the source and prevent
security issues associated with untrusted relays.
`RelayConnection.swift` — the source that initially handles WebSocket
messages — was analyzed, and measures were put in place to prevent
(or at least minimize) unverified nostr event data being spread
throughout the app.
The following measures were taken:
1. A note verification step was added prior to the `self.handleEvent(.nostr_event(ev))` call (which sends a Nostr response to the rest of the app for logical handling).
a. From code analysis, there is only one such call in `RelayConnection.swift`.
2. `NostrConnectionEvent`, the object that gets passed to event handlers, had its interface modified to remove the "message" case, since:
a. that could be a source of unverified nostr events.
b. it is redundant an unneeded due to the `.nostr_event` case.
c. there were no usages of it around the codebase
3. The raw websocket event handler had its label renamed to "handleUnverifiedWSEvent", to make it clear to the caller about the verification status of the data.
a. Usages of this were inspected and no significant risk was detected.
4. A new `verify` method in NdbNote was created to verify Nostr notes, and unit tests were added to confirm tampering detections around all the major fields in a Nostr note.
5. Care was taken to ensure the performance regression is as little as
possible.
It is worth noting that we will not need this once the local relay model
architecture is introduced, since that architecture ensures note
validation before it reaches the rest of the application and the user.
In other words, this is a temporary fix.
However, since the migration to that new architecture is a major
undertaking that will take some time to be completed, this fix was written
in order to address security concerns while the migration is unfinished.
This fix was written in a way that attempts to be as effective as
possible in reducing security risks without a risky and lenghty
refactor of the code that would delay the fix from being published.
Changelog-Fixed: Improved security around note validation
Closes: https://github.com/damus-io/damus/issues/1341
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit fixes a crash that occurred when swapping between Damus and
other apps.
When Damus enters background mode, NostrDB is closed and its resources
released. When Damus re-enters foreground mode, NostrDB is reopened.
However, an issue with the transaction inheritance logic
caused a race condition where a side menu profile lookup would get an
obsolete transaction containing pointers that have been freedwhen
NostrDB was closed, causing a "use-after-free" memory error.
The issue was fixed by improving the transaction inheritance logic to
double-check if the "generation" counter (which auto increments when
Damus closes and re-opens) matches the generation marked on the
thread-specific transaction. This effectively prevents lookups from
inheriting an obsolete transaction from a previous NostrDB generation.
Closes: https://github.com/damus-io/damus/issues/3167
Changelog-Fixed: Fixed an issue where the app would crash when swapping between apps
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
It was decided on a standup meeting that this feature is not important
and failing tests can be disabled.
Changelog-None
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit fixes a stack corruption issue caused by
an off-by-one error in one of the functions responsible
for parsing bech32 entities.
Changelog-None
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Previously two addresses from different memory regions were being
subtracted, which will lead to the incorrect number. This commit
improves the calculation.
Changelog-None
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Currently NostrDB does not seem to handle encryption/decryption of DMs.
Since NostrDB now controls the block parsing process and fetches note
contents directly from the database, we have to add a specific condition
that injects decrypted content directly to the ndb content parser.
This is done in conjunction with some minor refactoring to `NdbBlocks`
and associated structs, as in C those are separated between the content
string and the offsets for each block, but in Swift this is more
ergonomically represented as a standalone/self-containing object.
No changelog entry is added because the previously broken version was
never released to the public, and therefore this fix produces no
user-facing changes compared to the last released version.
Changelog-None
Closes: https://github.com/damus-io/damus/issues/3106
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
NostrDB relies on manual memory management, so it is a good idea to
enable the address sanitizer on debug configurations, as it helps find
memory-related issues on the app, which will allow us to identify memory
issues and potential crashes earlier in the development process.
Changelog-None
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit introduces new interfaces for working with NostrDB from
Swift, including `NostrFilter` conversion, subscription streaming via
AsyncStreams and lookup/wait functions.
No user-facing changes.
Changelog-None
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
I wanted to not amend this since we've already applied
it in the nostrdb-update branch on damus and I don't want
to conflict
Signed-off-by: William Casarin <jb55@jb55.com>
The Address Sanitizer detected a heap buffer overflow during a memcpy operation
in nostrdb.c associated with note parsing.
It was found that not enough memory was being allocated to the buffer to
support all the content parsing.
Allocation size was increased to support the memory needed for the
parsing operations. However, the new number was not carefully calculated
as we will not run into this code path once we switch to the local relay
model.
Changelog-Fixed: Fixed memory error in nostrdb
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Add support for type KIND for bech32-encoded entities naddr and nevent
as specified in NIP-19.
Co-authored-by: kernelkind <kernelkind@gmail.com>
Signed-off-by: William Casarin <jb55@jb55.com>
This adds support for nip50 fulltext searches. This allows you to use
the nostrdb query interface for executing fulltext searches instead of
the typical `ndb_text_search` api. The benefits of this include a
standardized query interface that also further filters on other fields
in the filter.
Changelog-Added: Add nip50 search filters and queries
Signed-off-by: William Casarin <jb55@jb55.com>
Update fulltext search queries to include an optional filter. This can
be used to narrow down the fulltext search. This is another step towards
nip50 support in nostrdb.
I noticed the code was exiting dubiously in certain situations... so we
fix that as well. It's possible we were missing search results because
of this.
Signed-off-by: William Casarin <jb55@jb55.com>
Add a helper for sorting search words from largest to smallest. This
should help search performance. For example, let's say our search index
is like so:
"the pokemon is cool"
the
the
the
...
* 1000
Our root word search would have to start 1000 new recursive queries. By
sorting by the largest word:
pokemon
pokemon
pokemon
...
* 10
We only have to do 10 recursive searches, assuming larger words are less
common, which will likely be the case most of the time
Signed-off-by: William Casarin <jb55@jb55.com>
This fixes an allocation issue with ndb_filter_init_with for small
page sizes. instead of allocating the buffer around pages, we allocate
based on total buffer size.
Fixes: f7aac3215575 ("filter: introduce ndb_filter_init_with")
Signed-off-by: William Casarin <jb55@jb55.com>
