Fix heap-buffer overflow from FPRDecodeString (#8631)

* Use trimmedURLString for logging

* Update CHANGELOG.md

* Move nanopb decode methods to test util

* Update Package.swift

* Incorporate comments

FirebasePerformance/CHANGELOG.md

@@ -1,6 +1,7 @@
 # Pending
 * Create a random number of delay for remote config fetch during app starts.
-* Fix log spamming when Firebase Performance is disabled. (#8423)
+* Fix log spamming when Firebase Performance is disabled. (#8423, #8577)
+* Fix heap-heap-buffer overflow when decoding strings. (#8628)
 
 # Version 8.6.1
 * Fix the case where the event were dropped for missing a critical field in the event.

FirebasePerformance/Sources/FPRClient.m

@@ -219,7 +219,7 @@
                                    : @"UNKNOWN";
       FPRLogInfo(kFPRClientMetricLogged,
                  @"Logging network request trace - %@, Response code: %@, %.4fms",
-                 FPRDecodeString(networkRequestMetric.url), responseCode, duration / 1000.0);
+                 trace.trimmedURLString, responseCode, duration / 1000.0);
       firebase_perf_v1_PerfMetric metric = FPRGetPerfMetricMessage(self.config.appID);
       FPRSetNetworkRequestMetric(&metric, networkRequestMetric);
       FPRSetApplicationProcessState(&metric,

FirebasePerformance/Sources/FPRNanoPbUtils.h

@@ -53,30 +53,6 @@ extern pb_bytes_array_t* _Nullable FPREncodeData(NSData* _Nonnull data);
  */
 extern pb_bytes_array_t* _Nullable FPREncodeString(NSString* _Nonnull string);
 
-/** Creates a NSData object by copying the given bytes array and returns the reference.
- *
- * @param pbData The pbData to dedoded as NSData
- * @return A reference to NSData
- */
-extern NSData* _Nullable FPRDecodeData(pb_bytes_array_t* _Nonnull pbData);
-
-/** Creates a NSString object by copying the given bytes array and returns the reference.
- *
- * @param pbData The pbData to dedoded as NSString
- * @return A reference to the NSString
- */
-extern NSString* _Nullable FPRDecodeString(pb_bytes_array_t* _Nonnull pbData);
-
-/** Creates a NSDictionary by copying the given bytes from the StringToStringMap object and returns
- * the reference.
- *
- * @param map The reference to a StringToStringMap object to be decoded.
- * @param count The number of entries in the dictionary.
- * @return A reference to the dictionary
- */
-extern NSDictionary<NSString*, NSString*>* _Nullable FPRDecodeStringToStringMap(
-    StringToStringMap* _Nullable map, NSInteger count);
-
 /** Callocs a nanopb StringToStringMap and copies the given NSDictionary bytes into the
  * StringToStringMap.
  *
@@ -85,16 +61,6 @@ extern NSDictionary<NSString*, NSString*>* _Nullable FPRDecodeStringToStringMap(
  */
 extern StringToStringMap* _Nullable FPREncodeStringToStringMap(NSDictionary* _Nullable dict);
 
-/** Creates a NSDictionary by copying the given bytes from the StringToNumberMap object and returns
- * the reference.
- *
- * @param map The reference to a StringToNumberMap object to be decoded.
- * @param count The number of entries in the dictionary.
- * @return A reference to the dictionary
- */
-extern NSDictionary<NSString*, NSNumber*>* _Nullable FPRDecodeStringToNumberMap(
-    StringToNumberMap* _Nullable map, NSInteger count);
-
 /** Callocs a nanopb StringToNumberMap and copies the given NSDictionary bytes into the
  * StringToStringMap.
  *

FirebasePerformance/Sources/FPRNanoPbUtils.m

@@ -150,16 +150,6 @@ pb_bytes_array_t *FPREncodeString(NSString *string) {
   return FPREncodeData(stringBytes);
 }
 
-NSData *FPRDecodeData(pb_bytes_array_t *pbData) {
-  NSData *data = [NSData dataWithBytes:&(pbData->bytes) length:pbData->size];
-  return data;
-}
-
-NSString *FPRDecodeString(pb_bytes_array_t *pbData) {
-  NSData *data = FPRDecodeData(pbData);
-  return [NSString stringWithCString:[data bytes] encoding:NSUTF8StringEncoding];
-}
-
 StringToStringMap *_Nullable FPREncodeStringToStringMap(NSDictionary *_Nullable dict) {
   StringToStringMap *map = calloc(dict.count, sizeof(StringToStringMap));
   __block NSUInteger index = 0;
@@ -171,17 +161,6 @@ StringToStringMap *_Nullable FPREncodeStringToStringMap(NSDictionary *_Nullable
   return map;
 }
 
-NSDictionary<NSString *, NSString *> *FPRDecodeStringToStringMap(StringToStringMap *map,
-                                                                 NSInteger count) {
-  NSMutableDictionary<NSString *, NSString *> *dict = [[NSMutableDictionary alloc] init];
-  for (int i = 0; i < count; i++) {
-    NSString *key = FPRDecodeString(map[i].key);
-    NSString *value = FPRDecodeString(map[i].value);
-    dict[key] = value;
-  }
-  return [dict copy];
-}
-
 StringToNumberMap *_Nullable FPREncodeStringToNumberMap(NSDictionary *_Nullable dict) {
   StringToNumberMap *map = calloc(dict.count, sizeof(StringToNumberMap));
   __block NSUInteger index = 0;
@@ -194,19 +173,6 @@ StringToNumberMap *_Nullable FPREncodeStringToNumberMap(NSDictionary *_Nullable
   return map;
 }
 
-NSDictionary<NSString *, NSNumber *> *_Nullable FPRDecodeStringToNumberMap(
-    StringToNumberMap *_Nullable map, NSInteger count) {
-  NSMutableDictionary<NSString *, NSNumber *> *dict = [[NSMutableDictionary alloc] init];
-  for (int i = 0; i < count; i++) {
-    if (map[i].has_value) {
-      NSString *key = FPRDecodeString(map[i].key);
-      NSNumber *value = [NSNumber numberWithLongLong:map[i].value];
-      dict[key] = value;
-    }
-  }
-  return [dict copy];
-}
-
 firebase_perf_v1_PerfSession *FPREncodePerfSessions(NSArray<FPRSessionDetails *> *sessions,
                                                     NSInteger count) {
   firebase_perf_v1_PerfSession *perfSessions = calloc(count, sizeof(firebase_perf_v1_PerfSession));

FirebasePerformance/Tests/Unit/FPRNanoPbUtilsTest.m

@@ -29,6 +29,7 @@
 #import "FirebasePerformance/Sources/Gauges/Memory/FPRMemoryGaugeData.h"
 
 #import "FirebasePerformance/Tests/Unit/FPRTestCase.h"
+#import "FirebasePerformance/Tests/Unit/FPRTestUtils.h"
 
 #import <OCMock/OCMock.h>
 

FirebasePerformance/Tests/Unit/FPRTestUtils.h

@@ -14,6 +14,7 @@
 
 #import <Foundation/Foundation.h>
 
+#import "FirebasePerformance/Sources/FPRNanoPbUtils.h"
 #import "FirebasePerformance/Sources/Timer/FIRTrace+Internal.h"
 #import "FirebasePerformance/Sources/Timer/FIRTrace+Private.h"
 
@@ -65,6 +66,41 @@ NS_ASSUME_NONNULL_BEGIN
 /** Creates a random GDTCOREvent object with network event. */
 + (GDTCOREvent *)createRandomNetworkGDTEvent:(NSString *)url;
 
+/** Creates a NSData object by copying the given bytes array and returns the reference.
+ *
+ * @param pbData The pbData to dedoded as NSData
+ * @return A reference to NSData
+ */
+extern NSData *_Nullable FPRDecodeData(pb_bytes_array_t *_Nonnull pbData);
+
+/** Creates a NSString object by copying the given bytes array and returns the reference.
+ *
+ * @param pbData The pbData to dedoded as NSString
+ * @return A reference to the NSString
+ * @note This method may cause heap-buffer overflow
+ */
+extern NSString *_Nullable FPRDecodeString(pb_bytes_array_t *_Nonnull pbData);
+
+/** Creates a NSDictionary by copying the given bytes from the StringToStringMap object and returns
+ * the reference.
+ *
+ * @param map The reference to a StringToStringMap object to be decoded.
+ * @param count The number of entries in the dictionary.
+ * @return A reference to the dictionary
+ */
+extern NSDictionary<NSString *, NSString *> *_Nullable FPRDecodeStringToStringMap(
+    StringToStringMap *_Nullable map, NSInteger count);
+
+/** Creates a NSDictionary by copying the given bytes from the StringToNumberMap object and returns
+ * the reference.
+ *
+ * @param map The reference to a StringToNumberMap object to be decoded.
+ * @param count The number of entries in the dictionary.
+ * @return A reference to the dictionary
+ */
+extern NSDictionary<NSString *, NSNumber *> *_Nullable FPRDecodeStringToNumberMap(
+    StringToNumberMap *_Nullable map, NSInteger count);
+
 @end
 
 NS_ASSUME_NONNULL_END

FirebasePerformance/Tests/Unit/FPRTestUtils.m

@@ -27,7 +27,7 @@ static NSInteger const kLogSource = 462;  // LogRequest_LogSource_Fireperf
 
 @implementation FPRTestUtils
 
-#pragma mark - Test utility methods
+#pragma mark - Retrieve bundle
 
 + (NSBundle *)getBundle {
 #if SWIFT_PACKAGE
@@ -37,6 +37,7 @@ static NSInteger const kLogSource = 462;  // LogRequest_LogSource_Fireperf
 #endif
 }
 
+#pragma mark - Create events and PerfMetrics
 + (FIRTrace *)createRandomTraceWithName:(NSString *)traceName {
   FIRTrace *trace = [[FIRTrace alloc] initWithName:traceName];
   [trace start];
@@ -153,4 +154,40 @@ static NSInteger const kLogSource = 462;  // LogRequest_LogSource_Fireperf
   return gdtEvent;
 }
 
+#pragma mark - Decode nanoPb pbData
+
+NSData *FPRDecodeData(pb_bytes_array_t *pbData) {
+  NSData *data = [NSData dataWithBytes:&(pbData->bytes) length:pbData->size];
+  return data;
+}
+
+NSString *FPRDecodeString(pb_bytes_array_t *pbData) {
+  NSData *data = FPRDecodeData(pbData);
+  return [NSString stringWithCString:[data bytes] encoding:NSUTF8StringEncoding];
+}
+
+NSDictionary<NSString *, NSString *> *FPRDecodeStringToStringMap(StringToStringMap *map,
+                                                                 NSInteger count) {
+  NSMutableDictionary<NSString *, NSString *> *dict = [[NSMutableDictionary alloc] init];
+  for (int i = 0; i < count; i++) {
+    NSString *key = FPRDecodeString(map[i].key);
+    NSString *value = FPRDecodeString(map[i].value);
+    dict[key] = value;
+  }
+  return [dict copy];
+}
+
+NSDictionary<NSString *, NSNumber *> *_Nullable FPRDecodeStringToNumberMap(
+    StringToNumberMap *_Nullable map, NSInteger count) {
+  NSMutableDictionary<NSString *, NSNumber *> *dict = [[NSMutableDictionary alloc] init];
+  for (int i = 0; i < count; i++) {
+    if (map[i].has_value) {
+      NSString *key = FPRDecodeString(map[i].key);
+      NSNumber *value = [NSNumber numberWithLongLong:map[i].value];
+      dict[key] = value;
+    }
+  }
+  return [dict copy];
+}
+
 @end

Package.swift

@@ -893,6 +893,9 @@ let package = Package(
       ],
       cSettings: [
         .headerSearchPath("../../.."),
+        .define("PB_FIELD_32BIT", to: "1"),
+        .define("PB_NO_PACKED_STRUCTS", to: "1"),
+        .define("PB_ENABLE_MALLOC", to: "1"),
       ]
     ),