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

NSCoding Cyber Security Issue Founding: any Remediation? #3692

Open
3 tasks done
chopeace opened this issue Mar 26, 2024 · 2 comments
Open
3 tasks done

NSCoding Cyber Security Issue Founding: any Remediation? #3692

chopeace opened this issue Mar 26, 2024 · 2 comments
Labels
cache Anything related to Cache System
Milestone

Comments

@chopeace
Copy link

New Issue Checklist

Issue Info

Info Value
Platform Name ios
Platform Version 15.0
SDWebImage Version 5.18.2
Integration Method cocoapods
Xcode Version Xcode 15.2
Repro rate all the time (100%)
Repro with our demo prj cyber security issue
Demo project link cyber security issue

Issue Description and Steps

thanks for the contribution on your git. we are currently using it to load a image; WebImage(url:
However, we found that there is some cyber security issue(NSCoding) on your git repository; if you search NSCoding in your repository, you can easily find them in your git.

Therefore,I just wonder if your organization have a plan to swift NSCoding to NSSecureCoding(Secure Object Archiving APIs or cache) in a near future; if you have don't have any capacity to fix soon, it would be appreciated if you could response saying that you don't have plan on it now. Then, we could keep using your repository; otherwise, we need to consider to drop it off unfortunately(that is not what we want). Looking forward to hearing from your organization.

Thanks for your time and your contribution to our development community is crucial.
Best Regards,
Peace Cho.

Here is the founding

The NSKeyedArchiver or NSKeyedUnarchiver methods used by the App are insecure because they are incompatible with the NSSecureCoding protocol. An attacker-controlled payload that is deserialized via these APIs may result in attacker-controlled code being executed.

NSCoding is an Objective-C protocol that interoperates with NSKeyedArchiver and NSKeyedUnarchiver. Together, these APIs allow serialization and deserialization of code objects. However, the NSKeyedUnarchiver methods used by the app, and the NSCoding protocol itself, do not verify the type of object upon deserialization. Thus, an attacker may craft a malicious payload that results in unexpected code being executed.

To mitigate this vulnerability, Apple introduced the NSSecureCoding protocol along with the following secure methods of NSKeyedArchiver and NSKeyedUnarchiver, which are robust against this type of attack:

// Secure NSKeyedUnarchiver methods
- (instancetype)initForReadingFromData:(NSData *)data error:(NSError **)error;
+ (id)unarchivedObjectOfClass:(Class)cls fromData:(NSData *)data error:(NSError **)error;
+ (id)unarchivedObjectOfClasses:(NSSet<Class> *)classes fromData:(NSData *)data error:(NSError **)error;

// Secure NSKeyedArchiver methods
- (instancetype)initRequiringSecureCoding:(BOOL)requiresSecureCoding;
+ (NSData *)archivedDataWithRootObject:(id)object requiringSecureCoding:(BOOL)requiresSecureCoding error:(NSError **)error;

These APIs protect against object substitution attacks by requiring the programmer to declare the expected type of object before deserialization completes. Thus, if an invalid object is deserialized, the error can be handled safely.

Apple provides more information in the WWDC20 session, 'Securing Your App'.

here is the

Remediation Guidance

Locate all the classes in the App that conform to NSCoding and migrate them to NSSecureCoding.
Then, replace the insecure usages of NSKeyedArchiver and NSKeyedUnarchiver with the secure APIs that perform error handling and validate the expected type of the deserialized objects.

Additionally, ensure all input data is validated before it is used, especially when dealing with data that becomes executable.

// Declare that your class conforms to NSSecureCoding
@interface MySecureObject : NSObject <NSSecureCoding>
@property (nonatomic, retain) NSDictionary *myData;
@end

@implementation MySecureObject
+ (BOOL)supportsSecureCoding {
    // Must override this class delegate method to reture YES
    return YES;
}
+ (MySecureObject*)deserializeFromData:(NSData*)data {
    // Inform the system of the object type we expect to be deserialized from the data
    // This method will return an error if the serialized data was invalid
    NSError* out_error = nil;
    [NSKeyedUnarchiver unarchivedObjectOfClass:[MySecureObject class] fromData:data error:&out_error];
    if (out_error != nil) {
        // Handle the error
        NSLog(@"Deserialization failed: %@", out_error);
    }
}
- (id)initWithCoder:(NSCoder *)decoder {
    if ((self = [super init])) {
        // When decoding sub-objects, use @selector(decodeObjectOfClass:forKey:)
        // This method will throw an exception if the deserialized object's class doesn't match the expected class
        self.myData = [decoder decodeObjectOfClass:[NSDictionary class] forKey:@"myData"];
    }
    return self;
}
- (void)encodeWithCoder:(NSCoder *)encoder {
    [encoder encodeObject:self.myData forKey:@"myData"];
}
@end
@chopeace
Copy link
Author

The most important question is that whether SDWebImage use NSCoding or not by default when user use WebImage(url:)?
Because based on my search with sd_extendedObject, it said, "By default, if the image is cached, we do not send request to query new metadata.".
Also, I found that latest SDWebImage is 5.19.1( 6 does not exist).

@dreampiggy
Copy link
Contributor

dreampiggy commented Mar 27, 2024

NO.

This is open-sourced project. You can see as SDK, we do not even set this sd_extendedObject property.

Only when you (The app developers) explicitly set the API, then it will trigger the NSCoding archive/unarchive logic

If you'd think it's a security issue, do not use this API (sd_extendedObject) at all.

@dreampiggy dreampiggy added the cache Anything related to Cache System label Mar 27, 2024
@dreampiggy dreampiggy added this to the Future milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cache Anything related to Cache System
Projects
None yet
Development

No branches or pull requests

2 participants