twilio / twilio-video-ios

Programmable Video SDK by Twilio
http://twilio.com/video
Other
64 stars 22 forks source link

Switch from Unsafe Serialization API to NSSecureCoding #252

Open cagatayemekci opened 1 year ago

cagatayemekci commented 1 year ago

Before filing an issue please check that the issue is not already addressed by the following:

If this is an issue with the SDK itself, file it here. If this is an issue with the QuickStart apps, please use video-quickstart-ios.

Please ensure that you are not sharing any Personally Identifiable Information(PII) or sensitive account information (API keys, credentials, etc.) when reporting an issue.

Description

NSCoding does not verify the type of object upon deserialization and therefore is vulnerable to object substitution attacks. The code can protect itself from these attacks by using NSSecureCoding instead: https://developer.apple.com/documentation/foundation/nssecurecoding?language=objc

Expected Behavior

Use NSSecureCoding instead of NSCoding

Versions

All relevant version information for the issue.

Video iOS SDK

[e.g. 5.4 via SPM]

Xcode

[e.g. 14.1]

iOS Version

[e.g. 15.6.1]

iOS Device

[e.g. iPhone 11 Pro Max]

piyushtank commented 1 year ago

@cagatayemekci Thanks for reaching out. Can you please elaborate the issue you are facing or you are suggesting on Video SDK? Thanks.

cagatayemekci commented 1 year ago

We are using data theorem for security issues. This application also controls the frameworks we use. This is the details

Details

The following binaries within the App contain code which leverages `NSCoding`, which is vulnerable to object substitution attacks: `NSCoding` is an Objective-C protocol designed to allow serialization and deserialization of code objects. However, this protocol does not verify the type of object upon deserialization. Thus, it is vulnerable to object substitution attacks. A maliciously crafted payload which is deserialized via the `NSCoding` protocol can result in attacker-controlled code being executed. Apple provides the [`NSSecureCoding` protocol](https://developer.apple.com/documentation/foundation/nssecurecoding?language=objc) which is robust against this type of attack. `NSSecureCoding` protects 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.

piyushtank commented 1 year ago

@cagatayemekci Thanks for the details. Can you please share which class in Video SDK you are trying to deserialize which could potentially have the problem? Also are you talking about JSON serialization/deserialization? I am not sure why would you do serialization/deserialization of Twilio video sdk class/objects. They are not in JSON. Can you please share code snippets.

cagatayemekci commented 1 year ago

@piyushtank The following classes in the binary conform to NSCoding: TVIRTCVideoCodecInfo

piyushtank commented 1 year ago

I was out for few weeks. TVIRTCVideoCodecInfo is using webrtc class RTCVideoCodecInfo. Since this object is not serializing/deserializing, its not a risk, however we have created a ticket internally to fix this in our webrtc code base. We will pick up the ticket when we upgrade to next webrtc version.