diff --git a/Pods b/Pods index ea60f60ea..527dca96c 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit ea60f60ea01bc51fc2434248890b494e37da98a5 +Subproject commit 527dca96c23f0ac15664e9762987ff017cabdf90 diff --git a/SignalServiceKit/src/Messages/DeviceSyncing/OWSIncomingSentMessageTranscript.m b/SignalServiceKit/src/Messages/DeviceSyncing/OWSIncomingSentMessageTranscript.m index b70dada69..81f13f9dc 100644 --- a/SignalServiceKit/src/Messages/DeviceSyncing/OWSIncomingSentMessageTranscript.m +++ b/SignalServiceKit/src/Messages/DeviceSyncing/OWSIncomingSentMessageTranscript.m @@ -45,8 +45,15 @@ NS_ASSUME_NONNULL_BEGIN _quotedMessage = [TSQuotedMessage quotedMessageForDataMessage:_dataMessage thread:_thread transaction:transaction]; _contact = [OWSContacts contactForDataMessage:_dataMessage transaction:transaction]; - _linkPreview = - [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:_dataMessage body:_body transaction:transaction]; + + NSError *linkPreviewError; + _linkPreview = [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:_dataMessage + body:_body + transaction:transaction + error:&linkPreviewError]; + if (linkPreviewError && ![OWSLinkPreview isNoPreviewError:linkPreviewError]) { + OWSLogError(@"linkPreviewError: %@", linkPreviewError); + } if (sentProto.unidentifiedStatus.count > 0) { NSMutableArray *nonUdRecipientIds = [NSMutableArray new]; diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index f5f0634be..716ea807a 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -4,6 +4,13 @@ import Foundation +@objc +public enum LinkPreviewError: Int, Error { + case invalidInput + case assertionFailure + case noPreview +} + @objc(OWSLinkPreview) public class OWSLinkPreview: MTLModel { @objc @@ -13,13 +20,13 @@ public class OWSLinkPreview: MTLModel { public var title: String? @objc - public var attachmentId: String? + public var imageAttachmentId: String? @objc - public init(urlString: String, title: String?, attachmentId: String?) { + public init(urlString: String, title: String?, imageAttachmentId: String?) { self.urlString = urlString self.title = title - self.attachmentId = attachmentId + self.imageAttachmentId = imageAttachmentId super.init() } @@ -34,28 +41,36 @@ public class OWSLinkPreview: MTLModel { try super.init(dictionary: dictionaryValue) } + @objc + public class func isNoPreviewError(_ error: Error) -> Bool { + guard let error = error as? LinkPreviewError else { + return false + } + return error == .noPreview + } + @objc public class func buildValidatedLinkPreview(dataMessage: SSKProtoDataMessage, body: String?, - transaction: YapDatabaseReadWriteTransaction) -> OWSLinkPreview? { + transaction: YapDatabaseReadWriteTransaction) throws -> OWSLinkPreview { guard let previewProto = dataMessage.preview else { - return nil + throw LinkPreviewError.noPreview } let urlString = previewProto.url guard URL(string: urlString) != nil else { - owsFailDebug("Could not parse preview URL.") - return nil + Logger.error("Could not parse preview URL.") + throw LinkPreviewError.invalidInput } guard let body = body else { - owsFailDebug("Preview for message without body.") - return nil + Logger.error("Preview for message without body.") + throw LinkPreviewError.invalidInput } let bodyComponents = body.components(separatedBy: .whitespacesAndNewlines) guard bodyComponents.contains(urlString) else { - owsFailDebug("URL not present in body.") - return nil + Logger.error("URL not present in body.") + throw LinkPreviewError.invalidInput } // TODO: Verify that url host is in whitelist. @@ -68,7 +83,8 @@ public class OWSLinkPreview: MTLModel { imageAttachmentPointer.save(with: transaction) imageAttachmentId = imageAttachmentPointer.uniqueId } else { - owsFailDebug("Could not parse image proto.") + Logger.error("Could not parse image proto.") + throw LinkPreviewError.invalidInput } } @@ -78,23 +94,101 @@ public class OWSLinkPreview: MTLModel { } let hasImage = imageAttachmentId != nil if !hasTitle && !hasImage { - owsFailDebug("Preview has neither title nor image.") - return nil + Logger.error("Preview has neither title nor image.") + throw LinkPreviewError.invalidInput } - return OWSLinkPreview(urlString: urlString, title: title, attachmentId: imageAttachmentId) + return OWSLinkPreview(urlString: urlString, title: title, imageAttachmentId: imageAttachmentId) } @objc public func removeAttachment(transaction: YapDatabaseReadWriteTransaction) { - guard let attachmentId = attachmentId else { + guard let imageAttachmentId = imageAttachmentId else { owsFailDebug("No attachment id.") return } - guard let attachment = TSAttachment.fetch(uniqueId: attachmentId, transaction: transaction) else { + guard let attachment = TSAttachment.fetch(uniqueId: imageAttachmentId, transaction: transaction) else { owsFailDebug("Could not load attachment.") return } attachment.remove(with: transaction) } + + // MARK: - Domain Whitelist + + private static let linkDomainWhitelist = [ + "youtube.com", + "reddit.com", + "imgur.com", + "instagram.com" + ] + + private static let mediaDomainWhitelist = [ + "ytimg.com", + "cdninstagram.com" + ] + + private static let protocolWhitelist = [ + "https" + ] + + @objc + public class func isValidLinkUrl(_ urlString: String) -> Bool { + guard let url = URL(string: urlString) else { + return false + } + return isUrlInDomainWhitelist(url: url, + domainWhitelist: OWSLinkPreview.linkDomainWhitelist) + } + + @objc + public class func isValidMediaUrl(_ urlString: String) -> Bool { + guard let url = URL(string: urlString) else { + return false + } + return isUrlInDomainWhitelist(url: url, + domainWhitelist: OWSLinkPreview.linkDomainWhitelist + OWSLinkPreview.mediaDomainWhitelist) + } + + private class func isUrlInDomainWhitelist(url: URL, domainWhitelist: [String]) -> Bool { + guard let urlProtocol = url.scheme?.lowercased() else { + return false + } + guard protocolWhitelist.contains(urlProtocol) else { + return false + } + guard let domain = url.host?.lowercased() else { + return false + } + // TODO: We need to verify: + // + // * The final domain whitelist. + // * The relationship between the "link" whitelist and the "media" whitelist. + // * Exact match or suffix-based? + // * Case-insensitive? + // * Protocol? + for whitelistedDomain in domainWhitelist { + if domain == whitelistedDomain.lowercased() || + domain.hasSuffix("." + whitelistedDomain.lowercased()) { + return true + } + } + return false + } + + // MARK: - Text Parsing + + @objc + public class func previewUrl(forMessageBodyText body: String?) -> String? { + guard let body = body else { + return nil + } + let components = body.components(separatedBy: .whitespacesAndNewlines) + for component in components { + if isValidLinkUrl(component) { + return component + } + } + return nil + } } diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.m b/SignalServiceKit/src/Messages/Interactions/TSMessage.m index e696aad10..a1a6979b2 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.m @@ -351,7 +351,7 @@ static const NSUInteger OWSMessageSchemaVersion = 4; [self.contactShare removeAvatarAttachmentWithTransaction:transaction]; } - if (self.linkPreview.attachmentId) { + if (self.linkPreview.imageAttachmentId) { [self.linkPreview removeAttachmentWithTransaction:transaction]; } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 9ad97da24..85ed660b3 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1287,10 +1287,15 @@ NS_ASSUME_NONNULL_BEGIN thread:oldGroupThread transaction:transaction]; + NSError *linkPreviewError; OWSLinkPreview *_Nullable linkPreview = [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:dataMessage body:body - transaction:transaction]; + transaction:transaction + error:&linkPreviewError]; + if (linkPreviewError && ![OWSLinkPreview isNoPreviewError:linkPreviewError]) { + OWSLogError(@"linkPreviewError: %@", linkPreviewError); + } OWSLogDebug(@"incoming message from: %@ for group: %@ with timestamp: %lu", envelopeAddress(envelope), @@ -1355,8 +1360,15 @@ NS_ASSUME_NONNULL_BEGIN thread:thread transaction:transaction]; + NSError *linkPreviewError; OWSLinkPreview *_Nullable linkPreview = - [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:dataMessage body:body transaction:transaction]; + [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:dataMessage + body:body + transaction:transaction + error:&linkPreviewError]; + if (linkPreviewError && ![OWSLinkPreview isNoPreviewError:linkPreviewError]) { + OWSLogError(@"linkPreviewError: %@", linkPreviewError); + } // Legit usage of senderTimestamp when creating incoming message from received envelope TSIncomingMessage *incomingMessage = diff --git a/SignalServiceKit/tests/Messages/OWSLinkPreviewTest.swift b/SignalServiceKit/tests/Messages/OWSLinkPreviewTest.swift new file mode 100644 index 000000000..b41d30ee0 --- /dev/null +++ b/SignalServiceKit/tests/Messages/OWSLinkPreviewTest.swift @@ -0,0 +1,157 @@ +// +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. +// + +import Foundation +import SignalServiceKit +import XCTest + +class OWSLinkPreviewTest: SSKBaseTestSwift { + + override func setUp() { + super.setUp() + // Put setup code here. This method is called before the invocation of each test method in the class. + } + + override func tearDown() { + // Put teardown code here. This method is called after the invocation of each test method in the class. + super.tearDown() + } + + func testBuildValidatedLinkPreview_TitleAndImage() { + let url = "https://www.youtube.com/watch?v=tP-Ipsat90c" + let body = "\(url)" + let previewBuilder = SSKProtoDataMessagePreview.builder(url: url) + previewBuilder.setTitle("Some Youtube Video") + let imageAttachmentBuilder = SSKProtoAttachmentPointer.builder(id: 1) + imageAttachmentBuilder.setKey(Randomness.generateRandomBytes(32)) + imageAttachmentBuilder.setContentType(OWSMimeTypeImageJpeg) + previewBuilder.setImage(try! imageAttachmentBuilder.build()) + let dataBuilder = SSKProtoDataMessage.builder() + dataBuilder.setPreview(try! previewBuilder.build()) + + self.readWrite { (transaction) in + XCTAssertNotNil(try! OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(), + body: body, + transaction: transaction)) + } + } + + func testBuildValidatedLinkPreview_Title() { + let url = "https://www.youtube.com/watch?v=tP-Ipsat90c" + let body = "\(url)" + let previewBuilder = SSKProtoDataMessagePreview.builder(url: url) + previewBuilder.setTitle("Some Youtube Video") + let dataBuilder = SSKProtoDataMessage.builder() + dataBuilder.setPreview(try! previewBuilder.build()) + + self.readWrite { (transaction) in + XCTAssertNotNil(try! OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(), + body: body, + transaction: transaction)) + } + } + + func testBuildValidatedLinkPreview_Image() { + let url = "https://www.youtube.com/watch?v=tP-Ipsat90c" + let body = "\(url)" + let previewBuilder = SSKProtoDataMessagePreview.builder(url: url) + let imageAttachmentBuilder = SSKProtoAttachmentPointer.builder(id: 1) + imageAttachmentBuilder.setKey(Randomness.generateRandomBytes(32)) + imageAttachmentBuilder.setContentType(OWSMimeTypeImageJpeg) + previewBuilder.setImage(try! imageAttachmentBuilder.build()) + let dataBuilder = SSKProtoDataMessage.builder() + dataBuilder.setPreview(try! previewBuilder.build()) + + self.readWrite { (transaction) in + XCTAssertNotNil(try! OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(), + body: body, + transaction: transaction)) + } + } + + func testBuildValidatedLinkPreview_NoTitleOrImage() { + let url = "https://www.youtube.com/watch?v=tP-Ipsat90c" + let body = "\(url)" + let previewBuilder = SSKProtoDataMessagePreview.builder(url: url) + let dataBuilder = SSKProtoDataMessage.builder() + dataBuilder.setPreview(try! previewBuilder.build()) + + self.readWrite { (transaction) in + do { + _ = try OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(), + body: body, + transaction: transaction) + XCTFail("Missing expected error.") + } catch { + // Do nothing. + } + } + } + + func testIsValidLinkUrl() { + XCTAssertTrue(OWSLinkPreview.isValidLinkUrl("https://www.youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertTrue(OWSLinkPreview.isValidLinkUrl("https://youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertTrue(OWSLinkPreview.isValidLinkUrl("https://www.youtube.com")) + + // Allow arbitrary subdomains. + XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://some.random.subdomain.youtube.com/watch?v=tP-Ipsat90c")) + + // Don't allow HTTP, only HTTPS + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("http://youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("mailto://youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("ftp://youtube.com/watch?v=tP-Ipsat90c")) + + // Don't allow similar domains. + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://xyoutube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://youtubex.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://youtube.comx/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://www.xyoutube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://www.youtubex.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://www.youtube.comx/watch?v=tP-Ipsat90c")) + + // Don't allow media domains. + XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://i.ytimg.com/vi/tP-Ipsat90c/maxresdefault.jpg")) + } + + func testIsValidMediaUrl() { + XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://www.youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://www.youtube.com")) + + // Allow arbitrary subdomains. + XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://some.random.subdomain.youtube.com/watch?v=tP-Ipsat90c")) + + // Don't allow HTTP, only HTTPS + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("http://youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("mailto://youtube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("ftp://youtube.com/watch?v=tP-Ipsat90c")) + + // Don't allow similar domains. + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://xyoutube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://youtubex.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://youtube.comx/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://www.xyoutube.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://www.youtubex.com/watch?v=tP-Ipsat90c")) + XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://www.youtube.comx/watch?v=tP-Ipsat90c")) + + // Allow media domains. + XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://i.ytimg.com/vi/tP-Ipsat90c/maxresdefault.jpg")) + } + + func testPreviewUrlForMessageBodyText() { + XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "")) + XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim")) + XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim http://")) + XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim http://a.com")) + + XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "https://www.youtube.com/watch?v=tP-Ipsat90c"), + "https://www.youtube.com/watch?v=tP-Ipsat90c") + XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim"), + "https://www.youtube.com/watch?v=tP-Ipsat90c") + + // If there are more than one, take the first. + XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim https://www.youtube.com/watch?v=other-url carol"), + "https://www.youtube.com/watch?v=tP-Ipsat90c") + } +}