Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed a crash on a malformed delete event #1743

Merged
merged 1 commit into from
Jan 29, 2025
Merged

Fixed a crash on a malformed delete event #1743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Added ability to delete lists. [#136](https://github.com/verse-pbc/issues/issues/136)
- Added analytics for feed source selection and lists. [#129](https://github.com/verse-pbc/issues/issues/129)
- Fixed: while searching for users to add to a list, NIP-05 searches dismiss the view. [#165](https://github.com/verse-pbc/issues/issues/165)
- Fixed a crash when processing a malformed delete (kind 5) event. [#170](https://github.com/verse-pbc/issues/issues/170)

### Internal Changes
- Added function for creating a new list and a test verifying list editing. [#112](https://github.com/verse-pbc/issues/issues/112)
Expand Down
4 changes: 4 additions & 0 deletions Nos.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,7 @@
C9CF23172A38A58B00EBEC31 /* ParseQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9CF23162A38A58B00EBEC31 /* ParseQueue.swift */; };
C9CF23182A38A58B00EBEC31 /* ParseQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9CF23162A38A58B00EBEC31 /* ParseQueue.swift */; };
C9D573492AB24B7300E06BB4 /* custom-xcassets.stencil in Resources */ = {isa = PBXBuildFile; fileRef = C9D573482AB24B7300E06BB4 /* custom-xcassets.stencil */; };
C9D846E92D43C7C900A71E30 /* malformed_list_delete.json in Resources */ = {isa = PBXBuildFile; fileRef = C9D846E82D43C7C900A71E30 /* malformed_list_delete.json */; };
C9DC6CBA2C1739AD00E1CFB3 /* View+HandleURLsInRouter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9DC6CB92C1739AD00E1CFB3 /* View+HandleURLsInRouter.swift */; };
C9DEBFD2298941000078B43A /* NosApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9DEBFD1298941000078B43A /* NosApp.swift */; };
C9DEBFD4298941000078B43A /* PersistenceController.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9DEBFD3298941000078B43A /* PersistenceController.swift */; };
Expand Down Expand Up @@ -1026,6 +1027,7 @@
C9CF23162A38A58B00EBEC31 /* ParseQueue.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ParseQueue.swift; sourceTree = "<group>"; };
C9D2839E2CB9B177007ADCB9 /* Nos 17.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Nos 17.xcdatamodel"; sourceTree = "<group>"; };
C9D573482AB24B7300E06BB4 /* custom-xcassets.stencil */ = {isa = PBXFileReference; lastKnownFileType = text; name = "custom-xcassets.stencil"; path = "Nos/Assets/SwiftGen Stencils/custom-xcassets.stencil"; sourceTree = SOURCE_ROOT; };
C9D846E82D43C7C900A71E30 /* malformed_list_delete.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = malformed_list_delete.json; sourceTree = "<group>"; };
C9DC6CB92C1739AD00E1CFB3 /* View+HandleURLsInRouter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+HandleURLsInRouter.swift"; sourceTree = "<group>"; };
C9DEBFCE298941000078B43A /* Nos.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Nos.app; sourceTree = BUILT_PRODUCTS_DIR; };
C9DEBFD1298941000078B43A /* NosApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NosApp.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1292,6 +1294,7 @@
03618AF72C82583D00BCBC55 /* Fixtures */ = {
isa = PBXGroup;
children = (
C9D846E82D43C7C900A71E30 /* malformed_list_delete.json */,
C9BD91882B61BBEF00FDA083 /* bad_contact_list.json */,
0350F1162C0A47B20024CC15 /* contact_list.json */,
03C853C52D03A50900164D6C /* follow_set.json */,
Expand Down Expand Up @@ -2379,6 +2382,7 @@
03618C982C826F2100BCBC55 /* text_note_with_media_metadata.json in Resources */,
032634682C10C0D600E489B5 /* nostr_build_nip96_response.json in Resources */,
03FE3F8C2C87BC9500D25810 /* text_note_multiple_media.json in Resources */,
C9D846E92D43C7C900A71E30 /* malformed_list_delete.json in Resources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
5 changes: 2 additions & 3 deletions Nos/Models/CoreData/Event+CoreDataClass.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,9 +514,8 @@ public class Event: NosManagedObject, VerifiableEvent {

for aTag in aTags.map({ $0[1] }) {
let components = aTag.split(separator: ":").map { String($0) }
let pubkey = components[1]

guard pubkey == author.hexadecimalPublicKey else {
guard let pubkey = components[safe: 1],
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, my fault. All direct array access should be done safely like this. Thanks!

pubkey == author.hexadecimalPublicKey else {
// ensure that this delete event only affects events with the author's pubkey
continue
}
Expand Down
1 change: 1 addition & 0 deletions NosTests/IntegrationTests/Fixtures/malformed_list_delete.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"id":"84efec01c8be4b3b30851e34f6851273273b669eaab79c2f7ae2681207e6ded2","pubkey":"d0a1ffb8761b974cec4a3be8cbcb2e96a7090dcf465ffeac839aa4ca20c9a59e","tags":[["a","9f7b7fd7ed2ec346e577d675612b3ecd60e0b6638870047ea7931968dabf70b6"]],"content":"List deleted by owner","sig":"9a61af200a7e9266855a9c3bc5bab25463fdc506a0d6f3f286704da4c4b53473205a895b6fd388b006ec2b357a6e0faf36263ba5279f9a8e3037f4c007bd9576","created_at":1697055962,"kind":5}
19 changes: 19 additions & 0 deletions NosTests/Models/CoreData/EventTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,23 @@ final class EventTests: CoreDataTestCase {
// Assert
XCTAssertEqual(references, [alice, bob])
}

// MARK: - Deletion

// This is a quick test for a crash I found while doing some debugging.
func test_trackDelete_givenMalformedListDeletionRequest() throws {
// Arrange
let jsonData = try jsonData(filename: "malformed_list_delete")
let jsonEvent = try JSONDecoder().decode(JSONEvent.self, from: jsonData)
let context = persistenceController.viewContext
let parsedEvent = try EventProcessor.parse(
jsonEvent: jsonEvent,
from: nil,
in: context,
skipVerification: true
)!

// Act & Assert
XCTAssertNoThrow(try parsedEvent.trackDelete(on: Relay(), context: context))
}
}