Make errorToCHIPErrorCode safer to use. (#18707)

The old setup was creating CHIP_ERROR instances pointing to filenames with non-static lifetime, which could cause use-after-free. Removes the workaround chip-tool-darwin had for the broken behavior.
project-chip · Jan 30, 2024 · 2655291 · 2655291
1 parent 3b9656c
commit 2655291
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 30 deletions.
diff --git a/examples/chip-tool-darwin/commands/common/CHIPCommandBridge.h b/examples/chip-tool-darwin/commands/common/CHIPCommandBridge.h
@@ -44,15 +44,7 @@ class CHIPCommandBridge : public Command
 
     void SetCommandExitStatus(CHIP_ERROR status)
     {
-#if CHIP_CONFIG_ERROR_SOURCE
-        // If there is a filename in the status makes a copy of the filename as the pointer may be released
-        // when the autorelease pool is drained.
-        strncpy(mCommandExitStatusFilename, status.GetFile(), sizeof(mCommandExitStatusFilename));
-        mCommandExitStatusFilename[sizeof(mCommandExitStatusFilename) - 1] = '\0';
-        mCommandExitStatus = chip::ChipError(status.AsInteger(), mCommandExitStatusFilename, status.GetLine());
-#else
         mCommandExitStatus = status;
-#endif // CHIP_CONFIG_ERROR_SOURCE
         ShutdownCommissioner();
         StopWaiting();
     }
@@ -90,9 +82,6 @@ class CHIPCommandBridge : public Command
     CHIP_ERROR ShutdownCommissioner();
     uint16_t CurrentCommissionerIndex();
 
-#if CHIP_CONFIG_ERROR_SOURCE
-    char mCommandExitStatusFilename[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
-#endif // CHIP_CONFIG_ERROR_SOURCE
     CHIP_ERROR mCommandExitStatus = CHIP_ERROR_INTERNAL;
 
     CHIP_ERROR StartWaiting(chip::System::Clock::Timeout seconds);

diff --git a/src/darwin/Framework/CHIP/CHIPError.mm b/src/darwin/Framework/CHIP/CHIPError.mm
@@ -28,6 +28,17 @@
 
 NSString * const MatterInteractionErrorDomain = @"MatterInteractionErrorDomain";
 
+// Class for holding on to a CHIP_ERROR that we can use as the value
+// in a dictionary.
+@interface CHIPErrorHolder : NSObject
+@property (nonatomic, readonly) CHIP_ERROR error;
+
+- (instancetype)init NS_UNAVAILABLE;
++ (instancetype)new NS_UNAVAILABLE;
+
+- (instancetype)initWithError:(CHIP_ERROR)error;
+@end
+
 @implementation CHIPError
 
 + (NSError *)errorForCHIPErrorCode:(CHIP_ERROR)errorCode
@@ -75,15 +86,7 @@ + (NSError *)errorForCHIPErrorCode:(CHIP_ERROR)errorCode
         }];
     }
 
-#if CHIP_CONFIG_ERROR_SOURCE
-    if (errorCode.GetFile()) {
-        userInfo[@"errorFile"] = [NSString stringWithUTF8String:errorCode.GetFile()];
-    }
-
-    if (errorCode.GetLine()) {
-        userInfo[@"errorLine"] = @(errorCode.GetLine());
-    }
-#endif // CHIP_CONFIG_ERROR_SOURCE
+    userInfo[@"underlyingError"] = [[CHIPErrorHolder alloc] initWithError:errorCode];
 
     return [NSError errorWithDomain:CHIPErrorDomain code:code userInfo:userInfo];
     ;
@@ -226,6 +229,13 @@ + (CHIP_ERROR)errorToCHIPErrorCode:(NSError * _Nullable)error
         return CHIP_ERROR_INTERNAL;
     }
 
+    if (error.userInfo != nil) {
+        id underlyingError = error.userInfo[@"underlyingError"];
+        if (underlyingError != nil && [underlyingError isKindOfClass:[CHIPErrorHolder class]]) {
+            return ((CHIPErrorHolder *) underlyingError).error;
+        }
+    }
+
     chip::ChipError::StorageType code;
     switch (error.code) {
     case CHIPErrorCodeInvalidStringLength:
@@ -261,17 +271,21 @@ + (CHIP_ERROR)errorToCHIPErrorCode:(NSError * _Nullable)error
     }
     }
 
-    const char * file = nullptr;
-    unsigned int line = 0;
-    if (error.userInfo != nil) {
-        if (error.userInfo[@"errorFile"] != nil) {
-            file = [error.userInfo[@"errorFile"] cStringUsingEncoding:NSUTF8StringEncoding];
-        }
-        if (error.userInfo[@"errorLine"] != nil) {
-            line = [error.userInfo[@"errorLine"] unsignedIntValue];
-        }
+    return chip::ChipError(code);
+}
+
+@end
+
+@implementation CHIPErrorHolder
+
+- (instancetype)initWithError:(CHIP_ERROR)error
+{
+    if (!(self = [super init])) {
+        return nil;
     }
-    return chip::ChipError(code, file, line);
+
+    _error = error;
+    return self;
 }
 
 @end