Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save & load the SPKI disk cache using secure coding. #212

Merged
merged 1 commit into from
Oct 22, 2019

Conversation

adamkaplan
Copy link
Contributor

Synopsis

NSSecureCoding encodes data about the original object that was serialized. During the de-serialization process, the secure coding APIs enforce that the created object is of the expected type. This is designed to prevent a substitution attacks in which a serialized file is modified in a way that allows the attacker to run arbitrary code with the privileges – and in the context – of the app.

Attack Details

This code was flagged by automated static security scanners at Yahoo. The vulnerability details are here: https://cwe.mitre.org/data/definitions/502.html

The scanner message:

Unsecure_Deserialization issue exists @ TrustKit/TrustKit/Pinning/TSKSPKIHashCache.m

Data from an untrusted input element dataWithContentsOfURL: at line 208 of TrustKit\TrustKit\Pinning\TSKSPKIHashCache.m is used to deserialize an object unarchiveObjectWithData: at line 208 of TrustKit\TrustKit\Pinning\TSKSPKIHashCache.m. While the class of unarchiveObjectWithData:object does NOT conform to NSSecureCoding protocol.

Proposed Fix

We figure this is easily solved by switching to the NSSecureCoding API introduced in iOS 11. The persisted data is of type NSMutableDictionary<NSData *, NSData *>, which already supports NSSecureCoding (both the object and the generic types).

Impact

Since the data is not currently persisted using a secure coding API, the de-serialization might fail. Specifically, -loadSPKICacheFromFileSystem may not succeed after the upgrade. This situation is identical to the "first launch" scenario. Upon initialization a new cache will be saved using the secure coding APIs and subsequent launches will operate as they currently do.

@nabla-c0d3 nabla-c0d3 merged commit d1c250d into datatheorem:master Oct 22, 2019
@nabla-c0d3
Copy link
Member

Thanks!

@adamkaplan adamkaplan deleted the secure-coding branch July 14, 2020 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants