diff --git a/FirebaseMessaging/CHANGELOG.md b/FirebaseMessaging/CHANGELOG.md index 68694c63d15..93e20e17717 100644 --- a/FirebaseMessaging/CHANGELOG.md +++ b/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). diff --git a/FirebaseMessaging/Sources/FIRMessagingRmqManager.m b/FirebaseMessaging/Sources/FIRMessagingRmqManager.m index a5499c7b7ce..6a28d1d926d 100644 --- a/FirebaseMessaging/Sources/FIRMessagingRmqManager.m +++ b/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; diff --git a/FirebaseMessaging/Tests/UnitTests/FIRMessagingRmqManagerTest.m b/FirebaseMessaging/Tests/UnitTests/FIRMessagingRmqManagerTest.m index 44b207444fd..ef08e31e4f5 100644 --- a/FirebaseMessaging/Tests/UnitTests/FIRMessagingRmqManagerTest.m +++ b/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. */