CVE-2017-5590: Origin Validation Error (CWE-346)
value: An incorrect implementation of "XEP-0280: Message Carbons" in multiple XMPP clients allows a remote attacker to impersonate any user, including contacts, in the vulnerable application's display. This allows for various kinds of social engineering attacks. This CVE is for ChatSecure (3.2.0 - 4.0.0; only iOS) and Zom (all versions up to 1.0.11; only iOS).
CWE description: The product does not properly verify that the source of data or communication is valid.
CVSS v3 base score: 5.9
Published: 2017-02-09T20:59Z
Repository: https://github.com/ChatSecure/ChatSecure-iOS
Language: Objective-C
Files changed: ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager.m, ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.h, ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.m
Commit date: 2017-01-23 13:16:45 -0800
Commit message: Changed to using carbon module delegate methods

Diff: diff --git a/ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager.m b/ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager.m
index 8e266f2c..c16770cc 100644
--- a/ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager.m
+++ b/ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager.m
@@ -217,18 +217,19 @@ - (void)setupStream
 	[self.xmppStream addDelegate:self delegateQueue:self.workQueue];
 	[self.xmppRoster addDelegate:self delegateQueue:self.workQueue];
     [self.xmppCapabilities addDelegate:self delegateQueue:self.workQueue];
     [self.xmppvCardTempModule addDelegate:self delegateQueue:self.workQueue];
     
-    // Message Carbons
-    _messageCarbons = [[XMPPMessageCarbons alloc] init];
-    [self.messageCarbons activate:self.xmppStream];
-    
     // Message storage
     _messageStorage = [[OTRXMPPMessageYapStroage alloc] initWithDatabaseConnection:self.databaseConnection];
     [self.messageStorage activate:self.xmppStream];
     
+    // Message Carbons
+    _messageCarbons = [[XMPPMessageCarbons alloc] init];
+    [self.messageCarbons addDelegate:self.messageStorage delegateQueue:self.messageStorage.moduleDelegateQueue];
+    [self.messageCarbons activate:self.xmppStream];
+    
     //Stream Management
     _streamManagementDelegate = [[OTRStreamManagementDelegate alloc] initWithDatabaseConnection:self.databaseConnection];
     
     //OTRStreamManagementYapStorage *streamManagementStorage = [[OTRStreamManagementYapStorage alloc] initWithDatabaseConnection:self.databaseConnection];
     XMPPStreamManagementMemoryStorage *memoryStorage = [[XMPPStreamManagementMemoryStorage alloc] init];
diff --git a/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.h b/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.h
index 8c161edf..b372fead 100644
--- a/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.h
+++ b/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.h
@@ -12,10 +12,11 @@
 
 NS_ASSUME_NONNULL_BEGIN
 @interface OTRXMPPMessageYapStroage : XMPPModule
 
 @property (nonatomic, strong, readonly) YapDatabaseConnection *databaseConnection;
+@property (nonatomic, readonly) dispatch_queue_t moduleDelegateQueue;
 
 /** This connection is only used for readWrites */
 - (instancetype)initWithDatabaseConnection:(YapDatabaseConnection *)databaseConnection;
 
 @end
diff --git a/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.m b/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.m
index 5c649250..d00fa0d5 100644
--- a/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.m
+++ b/ChatSecure/Classes/Controllers/XMPP/OTRXMPPMessageYapStroage.m
@@ -23,27 +23,15 @@ @implementation OTRXMPPMessageYapStroage
 
 - (instancetype)initWithDatabaseConnection:(YapDatabaseConnection *)connection
 {
     if (self = [self init]) {
         _databaseConnection = connection;
+        _moduleDelegateQueue = dispatch_queue_create("OTRXMPPMessageYapStroage-delegateQueue", 0);
     }
     return self;
 }
 
-- (void)xmppStream:(XMPPStream *)sender didReceiveMessage:(XMPPMessage *)xmppMessage
-{
-    [self storeMessage:xmppMessage stream:sender incoming:YES];
-}
-
-- (void)storeMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stream incoming:(BOOL)incoming
-{
-    if ([xmppMessage isMessageCarbon]) {
-        [self handleCarbonMessage:xmppMessage stream:stream];
-    } else {
-        [self handleMessage:xmppMessage stream:stream incoming:incoming];
-    }
-}
 
 - (OTRXMPPBuddy *)buddyForUsername:(NSString *)username stream:(XMPPStream *)stream transaction:(YapDatabaseReadTransaction *)transaction
 {
     return [OTRXMPPBuddy fetchBuddyWithUsername:username withAccountUniqueId:stream.tag transaction:transaction];
 }
@@ -74,20 +62,24 @@ - (OTROutgoingMessage *)outgoingMessageFromXMPPMessage:(XMPPMessage *)xmppMessag
 - (OTRIncomingMessage *)incomingMessageFromXMPPMessage:(XMPPMessage *)xmppMessage buddyId:(NSString *)buddyId
 {
     return (OTRIncomingMessage *)[self baseMessageFromXMPPMessage:xmppMessage buddyId:buddyId class:[OTRIncomingMessage class]];
 }
 
-- (void)handleMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stream incoming:(BOOL)incoming;
+- (void)xmppStream:(XMPPStream *)stream didReceiveMessage:(XMPPMessage *)xmppMessage
 {
     // We don't handle incoming group chat messages here
     // Check out OTRXMPPRoomYapStorage instead
     if ([[xmppMessage type] isEqualToString:@"groupchat"] ||
         [xmppMessage elementForName:@"x" xmlns:XMPPMUCUserNamespace] ||
         [xmppMessage elementForName:@"x" xmlns:@"jabber:x:conference"]) {
         return;
     }
     
+    if ([xmppMessage isMessageCarbon]) {
+        return;
+    }
+    
     [self.databaseConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
         if ([stream.tag isKindOfClass:[NSString class]]) {
             NSString *username = [[xmppMessage from] bare];
             
             [self handleChatState:xmppMessage username:username stream:stream transaction:transaction];
@@ -182,22 +174,20 @@ - (BOOL)duplicateMessage:(XMPPMessage *)message buddyUniqueId:(NSString *)buddyU
         }];
     }
     return result;
 }
 
-- (void)handleCarbonMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stream
+- (void)handleCarbonMessage:(XMPPMessage *)forwardedMessage stream:(XMPPStream *)stream outgoing:(BOOL)isOutgoing
 {
     //Sent Message Carbons are sent by our account to another
     //So from is our JID and to is buddy
-    BOOL incoming = NO;
-    XMPPMessage *forwardedMessage = [xmppMessage messageCarbonForwardedMessage];
+    BOOL incoming = !isOutgoing;
     
     
     NSString *username = nil;
-    if ([xmppMessage isReceivedMessageCarbon]) {
+    if (incoming) {
         username = [[forwardedMessage from] bare];
-        incoming = YES;
     } else {
         username = [[forwardedMessage to] bare];
     }
     
     [self.databaseConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction * __nonnull transaction) {
@@ -228,6 +218,13 @@ - (void)handleCarbonMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stre
             }
         }
     }];
 }
 
+#pragma - mark XMPPMessageCarbonsDelegate
+
+- (void)xmppMessageCarbons:(XMPPMessageCarbons *)xmppMessageCarbons didReceiveMessage:(XMPPMessage *)message outgoing:(BOOL)isOutgoing
+{
+    [self handleCarbonMessage:message stream:xmppMessageCarbons.xmppStream outgoing:isOutgoing];
+}
+
 @end

Vulnerable code:     // Message Carbons
    _messageCarbons = [[XMPPMessageCarbons alloc] init];
    [self.messageCarbons activate:self.xmppStream];
    
- (void)xmppStream:(XMPPStream *)sender didReceiveMessage:(XMPPMessage *)xmppMessage
{
    [self storeMessage:xmppMessage stream:sender incoming:YES];
}

- (void)storeMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stream incoming:(BOOL)incoming
{
    if ([xmppMessage isMessageCarbon]) {
        [self handleCarbonMessage:xmppMessage stream:stream];
    } else {
        [self handleMessage:xmppMessage stream:stream incoming:incoming];
    }
}
- (void)handleMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stream incoming:(BOOL)incoming;
- (void)handleCarbonMessage:(XMPPMessage *)xmppMessage stream:(XMPPStream *)stream
    BOOL incoming = NO;
    XMPPMessage *forwardedMessage = [xmppMessage messageCarbonForwardedMessage];
    if ([xmppMessage isReceivedMessageCarbon]) {
        incoming = YES;
Fixed code:     // Message Carbons
    _messageCarbons = [[XMPPMessageCarbons alloc] init];
    [self.messageCarbons addDelegate:self.messageStorage delegateQueue:self.messageStorage.moduleDelegateQueue];
    [self.messageCarbons activate:self.xmppStream];
    
@property (nonatomic, readonly) dispatch_queue_t moduleDelegateQueue;
        _moduleDelegateQueue = dispatch_queue_create("OTRXMPPMessageYapStroage-delegateQueue", 0);
- (void)xmppStream:(XMPPStream *)stream didReceiveMessage:(XMPPMessage *)xmppMessage
    if ([xmppMessage isMessageCarbon]) {
        return;
    }
    
- (void)handleCarbonMessage:(XMPPMessage *)forwardedMessage stream:(XMPPStream *)stream outgoing:(BOOL)isOutgoing
    BOOL incoming = !isOutgoing;
    if (incoming) {
#pragma - mark XMPPMessageCarbonsDelegate

- (void)xmppMessageCarbons:(XMPPMessageCarbons *)xmppMessageCarbons didReceiveMessage:(XMPPMessage *)message outgoing:(BOOL)isOutgoing
{
    [self handleCarbonMessage:message stream:xmppMessageCarbons.xmppStream outgoing:isOutgoing];
}