Bind rmqID instead of using stringWithFormat. (#14856)

Author: leojaygoogle

FirebaseMessaging/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fix a potential SQL injection issue. (#14846).
+
 # 11.9.0
 - [fixed] Migrate FCM codebase to new NSKeyedUnarchiver APIs. (#14424).
 

FirebaseMessaging/Sources/FIRMessagingRmqManager.m

@@ -277,14 +277,14 @@ - (int64_t)queryLastRmqId {
 - (FIRMessagingPersistentSyncMessage *)querySyncMessageWithRmqID:(NSString *)rmqID {
   __block FIRMessagingPersistentSyncMessage *persistentMessage;
   dispatch_sync(_databaseOperationQueue, ^{
-    NSString *queryFormat = @"SELECT %@ FROM %@ WHERE %@ = '%@'";
+    NSString *queryFormat = @"SELECT %@ FROM %@ WHERE %@ = ?";
     NSString *query =
         [NSString stringWithFormat:queryFormat,
                                    kSyncMessagesColumns,  // SELECT (rmq_id, expiration_ts,
                                                           // apns_recv, mcs_recv)
                                    kTableSyncMessages,    // FROM sync_rmq
-                                   kRmqIdColumn,          // WHERE rmq_id
-                                   rmqID];
+                                   kRmqIdColumn           // WHERE rmq_id
+    ];
 
     sqlite3_stmt *stmt;
     if (sqlite3_prepare_v2(self->_database, [query UTF8String], -1, &stmt, NULL) != SQLITE_OK) {
@@ -293,6 +293,13 @@ - (FIRMessagingPersistentSyncMessage *)querySyncMessageWithRmqID:(NSString *)rmq
       return;
     }
 
+    if (sqlite3_bind_text(stmt, 1, [rmqID UTF8String], (int)[rmqID length], SQLITE_STATIC) !=
+        SQLITE_OK) {
+      [self logError];
+      sqlite3_finalize(stmt);
+      return;
+    }
+
     const int rmqIDColumn = 0;
     const int expirationTimestampColumn = 1;
     const int apnsReceivedColumn = 2;

FirebaseMessaging/Tests/UnitTests/FIRMessagingRmqManagerTest.m

@@ -81,6 +81,21 @@ - (void)testSavingSyncMessage {
   XCTAssertFalse(persistentMessage.mcsReceived);
 }
 
+- (void)testQuerySyncMessageWithRmqID {
+  // This is to make sure there is no sql injection vulnerability.
+  // Otherwise, this would generate an SQL like this:
+  // SELECT ... FROM ... WHERE rmq_id = '' --';
+  // Which is a valid SQL and matches empty rmq_id.
+  NSString *rmqID = @"' --";
+  int64_t expirationTime = FIRMessagingCurrentTimestampInSeconds() + 1;
+  [self.rmqManager saveSyncMessageWithRmqID:rmqID expirationTime:expirationTime];
+
+  FIRMessagingPersistentSyncMessage *persistentMessage =
+      [self.rmqManager querySyncMessageWithRmqID:rmqID];
+  XCTAssertEqual(persistentMessage.expirationTime, expirationTime);
+  XCTAssertEqualObjects(persistentMessage.rmqID, rmqID);
+}
+
 /**
  *  Test updating a sync message initially received via MCS, now being received via APNS.
  */