Secure untrusted pointer dereference in EDOInvocationRequest and EDOMethodSignatureRequest

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

@@ -99,6 +99,11 @@ @interface EDOHostService ()
 @property(nonatomic, readonly) NSMutableDictionary<NSNumber *, id> *localObjects;
 /** The queue to update local objects atomically. */
 @property(nonatomic, readonly) dispatch_queue_t localObjectsSyncQueue;
+/**
+ * The ref counts for tracked objects in the service. The key is the address of a tracked object
+ * and the value is the count.
+ */
+@property(nonatomic, readonly) NSMutableDictionary<NSNumber *, NSNumber *> *localObjectRefCounts;
 /**
  * The tracked weak objects in the service. The key is the address of a tracked object and the
  * value is the object.
@@ -219,6 +224,7 @@ - (instancetype)initWithPort:(UInt16)port
   if (self) {
     _registeredToDevice = NO;
     _localObjects = [[NSMutableDictionary alloc] init];
+    _localObjectRefCounts = [[NSMutableDictionary alloc] init];
     _localObjectsSyncQueue =
         dispatch_queue_create("com.google.edo.service.localObjects", DISPATCH_QUEUE_SERIAL);
 
@@ -372,9 +378,20 @@ - (EDOObject *)distantObjectForLocalObject:(id)object hostPort:(EDOHostPort *)ho
   }
 
   NSNumber *objectKey = [NSNumber numberWithLongLong:(EDOPointerType)object];
+#if DEBUG
+  NSLog(@"[DEBUG] distantObjectForLocalObject: %p (key: %llx) in service: %p", object,
+        (unsigned long long)object, self);
+#endif
   if (object != self.rootLocalObject) {
     dispatch_sync(_localObjectsSyncQueue, ^{
+      NSNumber *countObj = [self.localObjectRefCounts objectForKey:objectKey];
+      NSUInteger count = countObj ? [countObj unsignedIntegerValue] : 0;
+      [self.localObjectRefCounts setObject:@(count + 1) forKey:objectKey];
+
       if (![self.localObjects objectForKey:objectKey]) {
+#if DEBUG
+        NSLog(@"[DEBUG] Adding object to localObjects: %p", object);
+#endif
         [self.localObjects setObject:object forKey:objectKey];
       }
     });
@@ -389,6 +406,27 @@ - (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) {
+    NSLog(@"[DEBUG] [%p] localObjectForAddress: %llx -> ROOT %@", self, remoteAddress,
+          _rootLocalObject);
+    return _rootLocalObject;
+  }
+  NSNumber *edoKey = [NSNumber numberWithLongLong:remoteAddress];
+  __block id object;
+  dispatch_sync(_localObjectsSyncQueue, ^{
+    object = self.localObjects[edoKey];
+  });
+
+  if (object) {
+    NSLog(@"[DEBUG] [%p] localObjectForAddress: %llx -> %@", self, remoteAddress, object);
+  } else {
+    NSLog(@"[DEBUG] [%p] localObjectForAddress: %llx -> NOT FOUND", self, remoteAddress);
+  }
+
+  return object;
+}
 
 - (BOOL)isObjectAliveWithPort:(EDOServicePort *)port remoteAddress:(EDOPointerType)remoteAddress {
   if (![_port match:port]) {
@@ -407,8 +445,17 @@ - (BOOL)isObjectAliveWithPort:(EDOServicePort *)port remoteAddress:(EDOPointerTy
 - (BOOL)removeObjectWithAddress:(EDOPointerType)remoteAddress {
   NSNumber *edoKey = [NSNumber numberWithLongLong:remoteAddress];
 
+  NSLog(@"[DEBUG] [%p] removeObjectWithAddress: %llx", self, remoteAddress);
+
   dispatch_sync(_localObjectsSyncQueue, ^{
-    [self.localObjects removeObjectForKey:edoKey];
+    NSNumber *countObj = [self.localObjectRefCounts objectForKey:edoKey];
+    NSUInteger count = countObj ? [countObj unsignedIntegerValue] : 0;
+    if (count > 1) {
+      [self.localObjectRefCounts setObject:@(count - 1) forKey:edoKey];
+    } else {
+      [self.localObjectRefCounts removeObjectForKey:edoKey];
+      [self.localObjects removeObjectForKey:edoKey];
+    }
   });
   return YES;
 }

Service/Sources/EDOInvocationMessage.m

@@ -146,8 +146,16 @@ 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 > 0 && exceptionStackTrace.count >= currentStackTrace.count) {
+    NSUInteger diff = exceptionStackTrace.count - currentStackTrace.count;
+    NSUInteger length = diff + 1;
+    if (length <= exceptionStackTrace.count) {
+      majorStackTrace = [exceptionStackTrace subarrayWithRange:NSMakeRange(0, length)];
+    }
+  }
+
   return [[EDORemoteException alloc] initWithName:[localException name]
                                            reason:[localException reason]
                                  callStackSymbols:majorStackTrace];
@@ -341,7 +349,25 @@ + (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.",
+                                     request.target];
+      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

@@ -28,6 +28,7 @@
 #import "Service/Sources/EDOObject+Private.h"
 #import "Service/Sources/EDOParameter.h"
 #import "Service/Sources/EDORemoteException.h"
+
 #import "Service/Sources/EDOServicePort.h"
 #import "Service/Sources/EDOServiceRequest.h"
 
@@ -59,9 +60,13 @@
   NSString *separationSymbol =
       [NSString stringWithFormat:@"|---- eDO invocation [%@ %@] ----|", classInfo, methodInfo];
 
-  NSMutableArray<NSString *> *fullStackTraces = [remoteException.callStackSymbols mutableCopy];
+  NSMutableArray<NSString *> *fullStackTraces = [[NSMutableArray alloc] init];
+  if (remoteException.callStackSymbols) {
+    [fullStackTraces addObjectsFromArray:remoteException.callStackSymbols];
+  }
   [fullStackTraces addObject:separationSymbol];
   [fullStackTraces addObjectsFromArray:localOutputStackTraces];
+
   return [[EDORemoteException alloc] initWithName:remoteException.name
                                            reason:remoteException.reason
                                  callStackSymbols:fullStackTraces];

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) {