How to use NSSecureCoding with id objects

Question

I'm creating a linked list and using containers to group the object, next, and previous properties. Like Foundation collections, I'd like it to implement NSSecureCoding. Here's the declaration:

@interface ListContainer : NSObject <NSCopying, NSSecureCoding>

@property (readonly, nonatomic) id object;
@property (nonatomic) ListContainer * next;
@property (nonatomic) ListContainer * previous;

@end

When implementing the - initWithCoder: method it hit me that I don't know what class to use for the object:

- (instancetype)initWithCoder:(NSCoder *)aDecoder
{
    self = [super init];

    if (self) {

        _object = [aDecoder decodeObjectOfClass:<#(__unsafe_unretained Class)#> forKey:@"object"];

        BOOL nextIsNil = [aDecoder decodeBoolForKey:@"nextIsNil"];

        if (!nextIsNil) {

            // Decode next
            _next = [aDecoder decodeObjectOfClass:[ListContainer class] forKey:@"next"];

            if (_next == nil) {
                return nil;
            }

            // Link the nodes manually to prevent infinite recursion
            self.next.previous = self;
        }
    }

    return self;
}

Should I use -decodeObjectForKey: instead? Is it still secure coding?

When you encode the object, you could have a private variable using NSStringFromClass using the id object's class name. When you decode it, use this to decode it. You can even encode it in a NSDictionary with a key/value pair rather separate variable, when decoding just use the className key value to instantiate the appropriate class and the value key value with the appropriate value. — TheCodingArt, Jul 12 '15 at 15:51
I did think of that. But if I understand correctly, secure coding is used to prevent instantiating objects of unknown types when the data comes from a foreign source (ie., online). If I do that, I still allow attackers to instantiate pretty much any object, as they can just encode whatever class they want in that key. They call this substitution attacks. I want my containers to be protected against that. — André Fratelli, Jul 12 '15 at 22:40
The NSDictionary that you would store the key/value pairs containing the designated information will be stored on disk using the secure encoding mechanism, meaning that the knowledge that the object is of type NSDictionary would have to be decoded before the knowledge of the key/value data is revealed. — TheCodingArt, Jul 13 '15 at 01:42
I can't agree with that. Firstly because it's not guaranteed that it'll stay on disk. Encoded objects are often used to be sent over the wire. Secondly because the knowledge of it being of type `NSDictionary` is explicitly written on the encoding. That would be a weak solution against attacks, I think. Breaking the encoding would be as simply as encoding a list myself and inspecting the binary result — André Fratelli, Jul 13 '15 at 12:09
Ignoring the fact that sending this information via network was not previously mentioned and the fact that on disk the NSDictionary would be using the secure encoding meaning that no one would be able to identify that the class is an NSDictionary due to the NSDictionary being encoded via: (Secondly because the knowledge of it being of type NSDictionary is explicitly written on the encoding) and even further that this could be masked by a custom container. There are other many ways to work around this that I'll note — TheCodingArt, Jul 13 '15 at 13:48
You have to get a bit creative off of what I have stated above rather expecting a direct solution for your particular problem. Note that what I said can still work even if you don't feel comfortable with it, if you take it out of context and use it's approach. You may use custom keys via the server to identify with local classes. This way you don't specify the class type, but you can map to a class type using the key value such as having a key for type = "list", subtype = "user", this could map to the UserList class. — TheCodingArt, Jul 13 '15 at 13:50
Just remember that you have to know the class. The only way around this is to have something that indicates a value that you can 100% identify as mapping to another class. If you want to get really complicated, you could subclass a group of classes that inherit from something generic, override the decodeWithClass method, and then throw a custom exception if it doesn't properly decode. This way you can create a custom parser that will catch an exception and try to decode for a new class every time an exception is thrown. — TheCodingArt, Jul 13 '15 at 13:52
I didn't mention sending over the wire because that's one of the main purposes of `NSSecureCoding` over `NSCoding`, I assumed it would be implicit: secure coding is safe. I still don't think that these work arounds are safe at all. Notice that Apple's classes such as `NSArray` implement secure coding. They don't limit the classes that can be encoded to known classes, it's generic. Isn't listing known/expected classes what secure coding does? How does Apple do it then? They don't limit which classes can be encoded. So either `NSArray` is lying or there's some real fix for this issue. — André Fratelli, Jul 13 '15 at 21:17
Notice that whatever I do, if I encode the class with the container it will always be weak. I can't think of a way in which the class comes from the encoded data and it is still safe. Also, I never said I would be the one using the container. It's actually for a framework so it must support pretty much any class anyone wants to put in there — André Fratelli, Jul 13 '15 at 21:19
As stated in the link below: "In “regular” coding, the name of the class that is created is **stored in the archive** and the unarchiver trusts it when it goes to allocate it during decoding." So I guess that storing the class as you suggest would actually be redundant. From: http://prod.lists.apple.com/archives/cocoa-dev/2014/Jun/msg00232.html — André Fratelli, Jul 16 '15 at 00:25

score 3 · Accepted Answer · answered Jul 16 '15 at 21:11

I ended up posting the same question to Cocoa's mailing list and the most interesting discussion happened. Some of the highlights:

[...] Make an NSArray of normal stuff, like NSString, NSNumber, encode it, decode it with decodeObjectForClasses, with no classes. You’ll fail on the array. Add the NSArray to the list of allowed classes and .. it works. So, you think, NSArray will blindly decode anything so it’s no-longer secure.

Add an object of a custom class which implements secure coding into the array, and it will start failing again. NSArray, and the other collection types, allow elements of known secure system types, like NSString, but fail at anything outside that. [...]

At this point I understand that NSArray doesn't behave as I expected. Secure coding doesn't seem so secure anymore:

This seems far from ideal [...] The fact that it decodes a set of classes known to implement NSSecureCoding is wrong, IMO, for two reasons [...]

1) The fact that the contained class implements NSSecureCoding does not mean that I'm expecting it. [...]

2) It limits the classes which can be stored. [...]

Getting a class that I'm not expecting in a substitution attack is especially dreadful. Apparently Cocoa's promise is different, though:

[...] if you use NSArray() or other collection classes directly in your coding, you need to check what you got back. They are ‘securely’ decoded to the extent that Apple believes decoding them will not result in a buffer overflow etc, that’s all you get by default. [...]

So, no, NSSecureCoding does not guarantee secure encoding of containers, or at least it doesn't guarantee type checking and you must do it yourself. Not even in Cocoa's native data structures as I initially assumed (with reason, I still think that).

Props go to Roland King for all the effort. You can see the full conversation here.

But that is Apple's implementation. Why do you not make your own implementation? — Léo Natan, Jul 16 '15 at 21:22
Sure, that's definitely an option. I just assumed that `NSSecureCoding` would handle situations like this, after all this is written in [the docs](https://developer.apple.com/library/prerelease/ios/documentation/Foundation/Reference/NSSecureCoding_Protocol_Ref/index.html) with respect to `NSCoding`: `This technique is potentially unsafe because by the time you can verify the class type, the object has already been constructed, and if this is part of a collection class, potentially inserted into an object graph`. That's how they introduce secure coding, so I guess it gives the wrong idea. — André Fratelli, Jul 16 '15 at 21:26
The Apple implementation is logical. `NSArray` is a generic container. It **cannot** check for object types because it is type agnostic. If you implement a list of `id` objects, you will hit the same problem. How do you test for types, at decode time, when you are meant to accept anything? So you end up with something akin to `[aDecoder decodeObjectOfClass:[NSObject class] forKey:@"next"];` which is meaningless for your purpose. What more, since encoded data can be manipulated by an attacker, storing object class information in the encoded data is also meaningless. — Léo Natan, Jul 16 '15 at 21:31
So what you get from the "secure coding" is a promise that you get a "safe" array object, rather than a misbehaving object. — Léo Natan, Jul 16 '15 at 21:32
Yeah, I know that now :) and sure, it makes sense, but I still think the docs are misleading. I didn't know whether Apple had implemented some workaround for this, so I though it was a good idea to dig deeper. Now that I know the facts I understand the whys and why nots — André Fratelli, Jul 16 '15 at 21:46

How to use NSSecureCoding with id objects

1 Answers1

Linked