Skip to content

Commit d44174e

Browse files
authored
Copy all C++ strings to NSString where they're not obviously safe (#893)
This fixes a known instances of memory corruption where in FSTLevelDBMutationQueue, the NSString view was retained for later, and the incorrect user was used, causing b/74381054. gRPC does not necessarily copy its string argumnets and if our hostname were configured to a non-default one it's possible that we could corrupt the host cache too. All remaining usages of util::WrapNSStringNoCopy are obviously safe: passed into logging or other known transient usages.
1 parent 622a911 commit d44174e

File tree

4 files changed

+7
-7
lines changed

4 files changed

+7
-7
lines changed

Firestore/Source/Local/FSTLevelDBMutationQueue.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ @implementation FSTLevelDBMutationQueue {
9494
+ (instancetype)mutationQueueWithUser:(const User &)user
9595
db:(std::shared_ptr<DB>)db
9696
serializer:(FSTLocalSerializer *)serializer {
97-
NSString *userID = user.is_authenticated() ? util::WrapNSStringNoCopy(user.uid()) : @"";
97+
NSString *userID = user.is_authenticated() ? util::WrapNSString(user.uid()) : @"";
9898

9999
return [[FSTLevelDBMutationQueue alloc] initWithUserID:userID db:db serializer:serializer];
100100
}
@@ -103,7 +103,7 @@ - (instancetype)initWithUserID:(NSString *)userID
103103
db:(std::shared_ptr<DB>)db
104104
serializer:(FSTLocalSerializer *)serializer {
105105
if (self = [super init]) {
106-
_userID = userID;
106+
_userID = [userID copy];
107107
_db = db;
108108
_serializer = serializer;
109109
}

Firestore/Source/Remote/FSTDatastore.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo *)databaseInfo
9292
credentials:(id<FSTCredentialsProvider>)credentials {
9393
if (self = [super init]) {
9494
_databaseInfo = databaseInfo;
95-
NSString *host = util::WrapNSStringNoCopy(databaseInfo->host());
95+
NSString *host = util::WrapNSString(databaseInfo->host());
9696
if (!databaseInfo->ssl_enabled()) {
9797
GRPCHost *hostConfig = [GRPCHost hostWithAddress:host];
9898
hostConfig.secure = NO;

Firestore/Source/Remote/FSTSerializerBeta.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ - (FSTResourcePath *)decodedQueryPath:(NSString *)name {
149149

150150
- (FSTResourcePath *)encodedResourcePathForDatabaseID:(const DatabaseId *)databaseID {
151151
return [FSTResourcePath pathWithSegments:@[
152-
@"projects", util::WrapNSStringNoCopy(databaseID->project_id()), @"databases",
153-
util::WrapNSStringNoCopy(databaseID->database_id())
152+
@"projects", util::WrapNSString(databaseID->project_id()), @"databases",
153+
util::WrapNSString(databaseID->database_id())
154154
]];
155155
}
156156

Firestore/Source/Remote/FSTStream.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ - (instancetype)initWithDatabase:(const DatabaseInfo *)database
629629
}
630630

631631
- (GRPCCall *)createRPCWithRequestsWriter:(GRXWriter *)requestsWriter {
632-
return [[GRPCCall alloc] initWithHost:util::WrapNSStringNoCopy(self.databaseInfo->host())
632+
return [[GRPCCall alloc] initWithHost:util::WrapNSString(self.databaseInfo->host())
633633
path:@"/google.firestore.v1beta1.Firestore/Listen"
634634
requestsWriter:requestsWriter];
635635
}
@@ -714,7 +714,7 @@ - (instancetype)initWithDatabase:(const DatabaseInfo *)database
714714
}
715715

716716
- (GRPCCall *)createRPCWithRequestsWriter:(GRXWriter *)requestsWriter {
717-
return [[GRPCCall alloc] initWithHost:util::WrapNSStringNoCopy(self.databaseInfo->host())
717+
return [[GRPCCall alloc] initWithHost:util::WrapNSString(self.databaseInfo->host())
718718
path:@"/google.firestore.v1beta1.Firestore/Write"
719719
requestsWriter:requestsWriter];
720720
}

0 commit comments

Comments
 (0)