Secure untrusted pointer dereference in EDOInvocationRequest and EDOMethodSignatureRequest

Service/Sources/EDOClientService.m

@@ -122,6 +122,52 @@ + (id)unwrappedObjectFromObject:(id)object {
         return resolvedInstance;
       }
     }
+    return object;
+  }
+
+  if ([objClass isSubclassOfClass:[NSArray class]]) {
+    BOOL modified = NO;
+    NSMutableArray<id> *newArray =
+        [NSMutableArray arrayWithCapacity:[((NSArray<id> *)object) count]];
+    for (id item in ((NSArray<id> *)object)) {
+      id unwrapped = [self unwrappedObjectFromObject:item];
+      if (unwrapped != item) {
+        modified = YES;
+      }
+      [newArray addObject:unwrapped ?: [NSNull null]];
+    }
+    return modified ? [newArray copy] : object;
+  }
+  if ([objClass isSubclassOfClass:[NSSet class]]) {
+    BOOL modified = NO;
+    NSMutableSet<id> *newSet = [NSMutableSet setWithCapacity:[((NSSet<id> *)object) count]];
+    for (id item in ((NSSet<id> *)object)) {
+      id unwrapped = [self unwrappedObjectFromObject:item];
+      if (unwrapped != item) {
+        modified = YES;
+      }
+      if (unwrapped) {
+        [newSet addObject:unwrapped];
+      }
+    }
+    return modified ? [newSet copy] : object;
+  }
+  if ([objClass isSubclassOfClass:[NSDictionary class]]) {
+    BOOL modified = NO;
+    NSMutableDictionary<id, id> *newDict =
+        [NSMutableDictionary dictionaryWithCapacity:[((NSDictionary<id, id> *)object) count]];
+    for (id key in ((NSDictionary<id, id> *)object)) {
+      id value = ((NSDictionary<id, id> *)object)[key];
+      id unwrappedKey = [self unwrappedObjectFromObject:key];
+      id unwrappedValue = [self unwrappedObjectFromObject:value];
+      if (unwrappedKey != key || unwrappedValue != value) {
+        modified = YES;
+      }
+      if (unwrappedKey && unwrappedValue) {
+        newDict[unwrappedKey] = unwrappedValue;
+      }
+    }
+    return modified ? [newDict copy] : object;
   }
 
   return object;
@@ -218,22 +264,79 @@ + (id)cachedEDOFromObjectUpdateIfNeeded:(id)object {
       [EDOBlockObject isBlock:object] ? [EDOBlockObject EDOBlockObjectFromBlock:object] : object;
   Class objClass = object_getClass(edoObject);
   if (objClass == [EDOObject class] || objClass == [EDOBlockObject class]) {
-    id localObject = [self distantObjectReferenceForRemoteAddress:edoObject.remoteAddress];
-    EDOObject *localEDO = localObject;
-    if ([EDOBlockObject isBlock:localObject]) {
-      localEDO = [EDOBlockObject EDOBlockObjectFromBlock:localEDO];
+    NSNumber *edoKey = [NSNumber numberWithLongLong:edoObject.remoteAddress];
+    __block id result = object;
+    dispatch_sync(self.edoSyncQueue, ^{
+      id localObject = [self.localDistantObjects objectForKey:edoKey];
+      EDOObject *localEDO = localObject;
+      if ([EDOBlockObject isBlock:localObject]) {
+        localEDO = [EDOBlockObject EDOBlockObjectFromBlock:localEDO];
+      }
+      // Verify the service in case the old address is overwritten by a new service.
+      if (localObject && [edoObject.servicePort match:localEDO.servicePort]) {
+        // Since we already have the EDOObject in the cache, mark the new decoded EDOObject
+        // as local so it does not send a ReleaseRequest upon deallocation.
+        edoObject.local = YES;
+        result = localObject;
+      } else {
+        if (localObject) {
+          // The old address was recycled/overwritten by a new service or object.
+          // Mark the old cached EDOObject as local so that when it deallocates,
+          // it does not send a ReleaseRequest that deregisters the new object at this address.
+          localEDO.local = YES;
+        }
+        // Track the new remote object.
+        [self.localDistantObjects setObject:object forKey:edoKey];
+      }
+    });
+    return result;
+  }
+
+  if ([objClass isSubclassOfClass:[NSArray class]]) {
+    BOOL modified = NO;
+    NSMutableArray<id> *newArray =
+        [NSMutableArray arrayWithCapacity:[((NSArray<id> *)object) count]];
+    for (id item in ((NSArray<id> *)object)) {
+      id cached = [self cachedEDOFromObjectUpdateIfNeeded:item];
+      if (cached != item) {
+        modified = YES;
+      }
+      [newArray addObject:cached ?: [NSNull null]];
     }
-    // Verify the service in case the old address is overwritten by a new service.
-    if ([edoObject.servicePort match:localEDO.servicePort]) {
-      // Since we already have the EDOObject in the cache, the new decoded EDOObject is
-      // taken as a temporary local object, which does not send release message.
-      edoObject.local = YES;
-      return localObject;
-    } else {
-      // Track the new remote object.
-      [self addDistantObjectReference:object];
+    return modified ? [newArray copy] : object;
+  }
+  if ([objClass isSubclassOfClass:[NSSet class]]) {
+    BOOL modified = NO;
+    NSMutableSet<id> *newSet = [NSMutableSet setWithCapacity:[((NSSet<id> *)object) count]];
+    for (id item in ((NSSet<id> *)object)) {
+      id cached = [self cachedEDOFromObjectUpdateIfNeeded:item];
+      if (cached != item) {
+        modified = YES;
+      }
+      if (cached) {
+        [newSet addObject:cached];
+      }
     }
+    return modified ? [newSet copy] : object;
   }
+  if ([objClass isSubclassOfClass:[NSDictionary class]]) {
+    BOOL modified = NO;
+    NSMutableDictionary<id, id> *newDict =
+        [NSMutableDictionary dictionaryWithCapacity:[((NSDictionary<id, id> *)object) count]];
+    for (id key in ((NSDictionary<id, id> *)object)) {
+      id value = ((NSDictionary<id, id> *)object)[key];
+      id cachedKey = [self cachedEDOFromObjectUpdateIfNeeded:key];
+      id cachedValue = [self cachedEDOFromObjectUpdateIfNeeded:value];
+      if (cachedKey != key || cachedValue != value) {
+        modified = YES;
+      }
+      if (cachedKey && cachedValue) {
+        newDict[cachedKey] = cachedValue;
+      }
+    }
+    return modified ? [newDict copy] : object;
+  }
+
   return object;
 }
 

Service/Sources/EDOHostService+Private.h

@@ -64,6 +64,18 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (BOOL)isObjectAliveWithPort:(EDOServicePort *)port remoteAddress:(EDOPointerType)remoteAddress;
 
+/**
+ * Resolves the local object that this service has previously vended for the given address.
+ *
+ * The address is treated purely as an opaque lookup key into the service's tracked-object table; it
+ * is never dereferenced. This is the safe replacement for casting a wire-supplied @c EDOPointerType
+ * back to @c id.
+ *
+ * @param remoteAddress The address that was previously returned to the client in an @c EDOObject.
+ * @return The tracked local object, or @c nil if @c remoteAddress is not known to this service.
+ */
+- (nullable id)localObjectForAddress:(EDOPointerType)remoteAddress;
+
 /**
  * Removes an EDOObject with the specified address in the host cache.
  *

Service/Sources/EDOHostService.m

@@ -389,6 +389,19 @@ - (EDOObject *)distantObjectForLocalObject:(id)object hostPort:(EDOHostPort *)ho
     return [EDOObject edo_remoteProxyFromUnderlyingObject:object withPort:port];
   }
 }
+- (id)localObjectForAddress:(EDOPointerType)remoteAddress {
+  // ivar is used directly here to avoid the service lazily creating the listen port.
+  if (_rootLocalObject && (EDOPointerType)_rootLocalObject == remoteAddress) {
+    return _rootLocalObject;
+  }
+  NSNumber *edoKey = [NSNumber numberWithLongLong:remoteAddress];
+  __block id object;
+  dispatch_sync(_localObjectsSyncQueue, ^{
+    object = self.localObjects[edoKey];
+  });
+
+  return object;
+}
 
 - (BOOL)isObjectAliveWithPort:(EDOServicePort *)port remoteAddress:(EDOPointerType)remoteAddress {
   if (![_port match:port]) {

Service/Sources/EDOInvocationMessage.m

@@ -146,8 +146,15 @@ static EDOMethodFamily MethodTypeOfRetainsReturn(const char *methodName, Class t
   }
   NSArray<NSString *> *exceptionStackTrace = [localException callStackSymbols];
   NSArray<NSString *> *currentStackTrace = [NSThread callStackSymbols];
-  NSArray<NSString *> *majorStackTrace = [exceptionStackTrace
-      subarrayWithRange:NSMakeRange(0, exceptionStackTrace.count - currentStackTrace.count + 1)];
+  NSArray<NSString *> *majorStackTrace = exceptionStackTrace;
+
+  if (exceptionStackTrace.count >= currentStackTrace.count) {
+    NSUInteger length = exceptionStackTrace.count - currentStackTrace.count + 1;
+    if (length <= exceptionStackTrace.count) {
+      majorStackTrace = [exceptionStackTrace subarrayWithRange:NSMakeRange(0, length)];
+    }
+  }
+
   return [[EDORemoteException alloc] initWithName:[localException name]
                                            reason:[localException reason]
                                  callStackSymbols:majorStackTrace];
@@ -341,7 +348,26 @@ + (EDORequestHandler)requestHandler {
     NSAssert([request isKindOfClass:[EDOInvocationRequest class]],
              @"EDOInvocationRequest is expected.");
     EDOHostPort *hostPort = request.hostPort;
-    id target = (__bridge id)(void *)request.target;
+    // The target address arrives off the wire as a raw 64-bit integer. It must NOT be cast to id
+    // until the service has confirmed it is an object it previously vended; otherwise an attacker
+    // can supply an arbitrary pointer and obtain a wild dereference / objc_msgSend on a fake isa.
+    id target = [service localObjectForAddress:request.target];
+    if (!target) {
+      NSString *reason = [NSString
+          stringWithFormat:@"The target address (%llx) is not tracked by this service (selector: "
+                           @"%@, servicePort: %u).",
+                           request.target, request.selectorName, service.port.hostPort.port];
+      EDORemoteException *remoteException =
+          [[EDORemoteException alloc] initWithName:EDOServiceGenericException
+                                            reason:reason
+                                  callStackSymbols:[NSThread callStackSymbols]];
+
+      return [EDOInvocationResponse responseWithReturnValue:nil
+                                                  exception:remoteException
+                                                  outValues:nil
+                                                 forRequest:request
+                                                targetClass:Nil];
+    }
     SEL sel = NSSelectorFromString(request.selectorName);
 
     EDOBoxedValueType *returnValue;

Service/Sources/EDOMethodSignatureMessage.m

@@ -16,6 +16,7 @@
 
 #import "Service/Sources/EDOMethodSignatureMessage.h"
 
+#import "Service/Sources/EDOHostService+Private.h"
 #import "Service/Sources/EDOHostService.h"
 #import "Service/Sources/EDOMessage.h"
 #import "Service/Sources/EDOObject+Private.h"
@@ -97,7 +98,12 @@ + (EDORequestHandler)requestHandler {
     }
 
     EDOMethodSignatureRequest *methodRequest = (EDOMethodSignatureRequest *)request;
-    id object = (__bridge Class)(void *)methodRequest.object;
+    id object = [service localObjectForAddress:methodRequest.object];
+    if (!object) {
+      // If the object is not found, we can't get method signature.
+      // Returning nil signature will eventually result in an exception at the client.
+      return [[EDOMethodSignatureResponse alloc] initWithSignature:nil forRequest:request];
+    }
     SEL sel = NSSelectorFromString(methodRequest.selectorName);
 
     NSMethodSignature *signature = EDOGetMethodSignature(object, sel);

Service/Sources/EDOObject+Invocation.m

@@ -59,7 +59,8 @@
   NSString *separationSymbol =
       [NSString stringWithFormat:@"|---- eDO invocation [%@ %@] ----|", classInfo, methodInfo];
 
-  NSMutableArray<NSString *> *fullStackTraces = [remoteException.callStackSymbols mutableCopy];
+  NSMutableArray<NSString *> *fullStackTraces =
+      [remoteException.callStackSymbols mutableCopy] ?: [[NSMutableArray alloc] init];
   [fullStackTraces addObject:separationSymbol];
   [fullStackTraces addObjectsFromArray:localOutputStackTraces];
   return [[EDORemoteException alloc] initWithName:remoteException.name

Service/Tests/UnitTests/EDOMessageTest.m

@@ -435,6 +435,13 @@ - (void)testClassMethodInvocationHandler {
   EDOTestDummy *dummyLocal = [[EDOTestDummy alloc] init];
   [self edo_createQueueAndServiceWithRootObject:dummyLocal
                                           block:^(EDOHostService *service) {
+                                            // Simulate the client asking for the class first to
+                                            // register it securely.
+                                            EDOServiceRequest *classRequest = [EDOClassRequest
+                                                requestWithClassName:@"EDOTestDummy"
+                                                            hostPort:service.port.hostPort];
+                                            EDOClassRequest.requestHandler(classRequest, service);
+
                                             EDOInvocationResponse *response = [self
                                                 edo_runInvocationWithService:service
                                                                       target:[dummyLocal class]
@@ -507,7 +514,7 @@ - (void)testMethodSignatureRequestHandler {
   void *remoteAddress = (__bridge void *)dummyLocal;
 
   EDOHostService *service = [EDOHostService serviceWithPort:0
-                                                 rootObject:self
+                                                 rootObject:dummyLocal
                                                       queue:dispatch_get_main_queue()];
 
   [EDOTestDummy enumerateSelector:^(SEL selector) {