Integer overflow check in memory allocation

[adamkaplan]

Synopsis

The copy of RSSwizzle embedded in TrustKit uses an unchecked outside value to allocate memory. The PR adds integer overflow protection to the memory allocation size.

Attack Details

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

The scanner message:

Signed_Memory_Arithmetic issue exists @ TrustKit/TrustKit/Dependencies/RSSwizzle/RSSwizzle.m
Signed integer strlen at line 73 of TrustKit\TrustKit\Dependencies\RSSwizzle\RSSwizzle.m specifies size of memory to allocate.

Proposed Fix

Added a simple integer overflow check to the code: if (strlen(x) > strlen(..)+2) {...}. When this case is true, the Swizzling is aborted.

Impact

There is no realistic failure path that doesn't impact the library. In the event of integer overflow in the objective-c type specifier, I added return NO, effectively declining to swizzle the method. The alternative is to allow the integer overflow, which will almost certainly crash the containing program. I would say that the impact of having this fix is "less severe" than not having it.

[nabla-c0d3]

Thanks for the PR! I'm curious: can you say which tool was used to find this?

[adamkaplan]

I'm really not too sure what they're using. The scanner automatically generates Jira tickets that get assigned to the responsible team. The Mitre link was provided in the ticket.