CVE-2017-5590: Improper Input Validation (CWE-20)
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 receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
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];
}