diff --git a/Service/Sources/EDOClientService.m b/Service/Sources/EDOClientService.m index e00f80c..2e91265 100644 --- a/Service/Sources/EDOClientService.m +++ b/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 *newArray = + [NSMutableArray arrayWithCapacity:[((NSArray *)object) count]]; + for (id item in ((NSArray *)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 *newSet = [NSMutableSet setWithCapacity:[((NSSet *)object) count]]; + for (id item in ((NSSet *)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 *newDict = + [NSMutableDictionary dictionaryWithCapacity:[((NSDictionary *)object) count]]; + for (id key in ((NSDictionary *)object)) { + id value = ((NSDictionary *)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 *newArray = + [NSMutableArray arrayWithCapacity:[((NSArray *)object) count]]; + for (id item in ((NSArray *)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 *newSet = [NSMutableSet setWithCapacity:[((NSSet *)object) count]]; + for (id item in ((NSSet *)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 *newDict = + [NSMutableDictionary dictionaryWithCapacity:[((NSDictionary *)object) count]]; + for (id key in ((NSDictionary *)object)) { + id value = ((NSDictionary *)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; } diff --git a/Service/Sources/EDOHostService+Private.h b/Service/Sources/EDOHostService+Private.h index 08d98eb..33d3e23 100644 --- a/Service/Sources/EDOHostService+Private.h +++ b/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. * diff --git a/Service/Sources/EDOHostService.m b/Service/Sources/EDOHostService.m index 0a8c33a..debb88b 100644 --- a/Service/Sources/EDOHostService.m +++ b/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]) { diff --git a/Service/Sources/EDOInvocationMessage.m b/Service/Sources/EDOInvocationMessage.m index d273f0e..36f4c32 100644 --- a/Service/Sources/EDOInvocationMessage.m +++ b/Service/Sources/EDOInvocationMessage.m @@ -146,8 +146,15 @@ static EDOMethodFamily MethodTypeOfRetainsReturn(const char *methodName, Class t } NSArray *exceptionStackTrace = [localException callStackSymbols]; NSArray *currentStackTrace = [NSThread callStackSymbols]; - NSArray *majorStackTrace = [exceptionStackTrace - subarrayWithRange:NSMakeRange(0, exceptionStackTrace.count - currentStackTrace.count + 1)]; + NSArray *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; diff --git a/Service/Sources/EDOMethodSignatureMessage.m b/Service/Sources/EDOMethodSignatureMessage.m index 2e39c6d..a0f16a4 100644 --- a/Service/Sources/EDOMethodSignatureMessage.m +++ b/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); diff --git a/Service/Sources/EDOObject+Invocation.m b/Service/Sources/EDOObject+Invocation.m index 238fcf5..139b9b0 100644 --- a/Service/Sources/EDOObject+Invocation.m +++ b/Service/Sources/EDOObject+Invocation.m @@ -59,7 +59,8 @@ NSString *separationSymbol = [NSString stringWithFormat:@"|---- eDO invocation [%@ %@] ----|", classInfo, methodInfo]; - NSMutableArray *fullStackTraces = [remoteException.callStackSymbols mutableCopy]; + NSMutableArray *fullStackTraces = + [remoteException.callStackSymbols mutableCopy] ?: [[NSMutableArray alloc] init]; [fullStackTraces addObject:separationSymbol]; [fullStackTraces addObjectsFromArray:localOutputStackTraces]; return [[EDORemoteException alloc] initWithName:remoteException.name diff --git a/Service/Tests/UnitTests/EDOMessageTest.m b/Service/Tests/UnitTests/EDOMessageTest.m index a454d33..a0e3cef 100644 --- a/Service/Tests/UnitTests/EDOMessageTest.m +++ b/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) {