Enforcing types with NSSecureCoding

Question

I decided to use NSSecureCoding over NSCoding, but I'm having trouble getting it to work.

I would expect the following code to fail, since I'm encoding an NSString but attempting to decode an NSNumber. The object is initialized without throwing an exception, however.

+ (BOOL)supportsSecureCoding
{
    return YES;
}

- (instancetype)initWithCoder:(NSCoder *)coder
{
    // prints '1' as expected
    NSLog(@"%d", coder.requiresSecureCoding);

    // unexpectedly prints 'foo' (expecting crash)
    NSLog(@"%@", [coder decodeObjectOfClass:NSNumber.class forKey:@"bar"]);

    return [super init];
}

- (void)encodeWithCoder:(NSCoder *)coder
{
    [coder encodeObject:@"foo" forKey:@"bar"];
}

Here's the code I'm using to test the snippet from above:

MyClass *object = [[MyClass alloc] init];

NSMutableData *const data = [[NSMutableData alloc] init];
NSKeyedArchiver *const archiver = [[NSKeyedArchiver alloc] initForWritingWithMutableData:data];
archiver.requiresSecureCoding = YES;

[archiver encodeObject:object forKey:@"root"];
[archiver finishEncoding];

NSKeyedUnarchiver *const unarchiver = [[NSKeyedUnarchiver alloc] initForReadingWithData:data];
unarchiver.requiresSecureCoding = YES;

[unarchiver decodeObjectOfClass:MyClass.class forKey:@"root"];
[unarchiver finishDecoding];

Am I missing something completely obvious or why is no exception thrown during decoding?

Acyclic

do your NSLogs actually print out the values? – Kyliekylila

Yes they do, they print what the comment above states. – Acyclic

Does the issue persist if you call [super initWithCoder:] instead of [super init]? – Robotize

@AaronBrager NSObject doesn't implement NSCoding afaik. I assume the call would crash my app. But I'll give it a try. Edit: calling [super initWithCoder:] won't even let my app compile. Besides, initialization shouldn't even matter in this context - I just want to retrieve a value from the coder. – Acyclic

This appears to just be failing on certain primitive types. If you do [@"foo" mutableCopy] or @[] you trigger a crash. Similarly @100 expecting NSString.class doesn't. I will write up a full answer tomorrow but I suspect this has to do with how low level types are stored in the backing plist. (Strings are a plist data type and have no security risk for decoding.) – Introversion

NSString and NSNumber aren't primitive types. Secure coding should prevent exactly what the OP is accomplishing. – Robotize

@AaronBrager You're right. The language choice was poor. I mean to say they're very basic types which are treated differently in the underlying storage of the coder so their behavior suggests something special is happening with them. This is definitely a bug. – Introversion

The decoding also "works" if you pass MyClass.class instead of NSNumber.class. Looks like some of the Foundation classes are "compatible" with all other classes. – Manheim

Answer 1

Looking at the definition of -[NSCoder decodeObjectOfClass:forKey:], yes, your code sample should have thrown an exception. The method's description says it:

Decodes an object for the key, restricted to the specified class.

And the discussion says:

If the coder responds YES to requiresSecureCoding, then an exception will be thrown if the class to be decoded does not implement NSSecureCoding or is not isKindOfClass: of aClass.

There are two inconsistencies with NSKeyedUnarchiver's implementation of this method, related to optimizations it does. The first is that decodeObjectOfClass:forKey: and decodeObjectForKey: only decode an object the first time it is encountered.

For example, the assert in the following code passes because foo and foo2 started out as the same object and were decoded just once while foo3 started as a separate object and as a result decoded separately.

func encodeWithCoder(coder:NSCoder) {
    let foo = NSSet(objects: 1, 2, 3)
    coder.encodeObject(foo, forKey: "foo")
    coder.encodeObject(foo, forKey: "foo2")
    coder.encodeObject(NSSet(objects: 1, 2, 3), forKey: "foo3")
}

required init(coder: NSCoder) {
    let foo = coder.decodeObjectOfClass(NSSet.self, forKey: "foo")
    let foo2 = coder.decodeObjectOfClass(NSSet.self, forKey: "foo2")
    let foo3 = coder.decodeObjectOfClass(NSSet.self, forKey: "foo3")
    assert(foo === foo2)
    assert(foo !== foo3)
    super.init()
}

It appears that classes are only checked when the object is actually being decoded. The list of approved classes is compared against the class the object is requesting. So in my previous example, I could change the class for foo2 to be whatever I want and the code will still run and return an NSSet:

required init(coder: NSCoder) {
    let foo = coder.decodeObjectOfClass(NSSet.self, forKey: "foo")
    let foo2 = coder.decodeObjectOfClass(NSMutableDictionary.self, forKey: "foo2")
    assert(foo === foo2)
    super.init()
}

The second inconsistency, related directly to your example, is that certain object types are never actually decoded. NSKeyedArchiver stores all its data as a binary property list, which according to Apple's source code has native support for string, data, number, date, dictionary, and array types. When NSKeyedArchiver encounters an NSString, NSNumber, or NSData object (but not a subclass), instead of encoding it using encodeWithObject: and saving information about how to decode it, it just stores the value directly in the PList. Then when you call decodeObjectOfClass:withKey: it sees the already present string and returns it right away without decoding. No decoding means no class check.

Whether this behavior is good or bad could be debated. Less checking means faster code but the behavior really doesn't match the API documentation. That said, you may be wondering what secure coding gets you if it doesn't guarantee return types. What secure coding with NSKeyedUnarchiver protects you from is that a maliciously crafted archive can't get you to call alloc/initWithCoder: on an arbitrary class. If you want more than that you could create a subclass that validates the output type of all decodeObjectOfClass:withKey: and decodeObjectOfClasses:withKey: call.

Introversion

Thanks for your detailed explanation. Too bad Apple's implementation isn't consistent with the documentation. – Acyclic

Brian writes, " decodeObjectOfClass:forKey: and decodeObjectForKey: only decode an object the first time it is encountered." I found this useful because I had SecureCoding set to YES and tried decodeObjectForKey: with a following decodeObjectOfClass:forKey: which both failed (same key). So, don't even try to decode insecurely before using the secure method (decodeObjectOfClass:forKey:). +1 for giving me the clue to why my code was failing. – Brittney