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

Crash in getPinningConfigurationKeyForDomain #210

Closed
GevaZeichner opened this issue Sep 25, 2019 · 4 comments
Closed

Crash in getPinningConfigurationKeyForDomain #210

GevaZeichner opened this issue Sep 25, 2019 · 4 comments

Comments

@GevaZeichner
Copy link

Hi,

A few of my users are getting NSRangeException.
TrustKit 1.6.1

Fatal Exception: NSRangeException
*** -[__NSCFString substringToIndex:]: Index 18446744073709551615 out of bounds; string length 16

Fatal Exception: NSRangeException
0  CoreFoundation                 0x20ea1927c __exceptionPreprocess
1  libobjc.A.dylib                0x20dbf39f8 objc_exception_throw
2  CoreFoundation                 0x20e9234b0 -[NSCache init]
3  Foundation                     0x20f3733a4 -[NSString substringToIndex:]
4  TrustKit                       0x108af3628 getPinningConfigurationKeyForDomain + 39 (configuration_utils.m:39)
5  TrustKit                       0x108afa144 -[TSKPinningValidator evaluateTrust:forHostname:] + 99 (TSKPinningValidator.m:99)
6  TrustKit                       0x108afa6a8 -[TSKPinningValidator handleChallenge:completionHandler:] + 203 (TSKPinningValidator.m:203)
@GevaZeichner
Copy link
Author

GevaZeichner commented Sep 30, 2019

Hi @nabla-c0d3, I tried debugging this and it is not clear how this situation is possible. Looking on configuration_utils.m:39, in order for the crash to happen, it seems that [subdomain length] would need to be equal to domainRegistryLength. Any ideas? Should we just add another safeguard checking these values, or is there a better fix? Can it be related to the user's network, maybe they have weird redirects or something of that sort?

@nabla-c0d3
Copy link
Member

Hello - do you know for which domain this is happening? Or maybe a list of possible domains?
Thanks!

@GevaZeichner
Copy link
Author

Hi @nabla-c0d3! We don't know exactly, but I suspect it might be a subdomain of s3.amazonaws.com (length 16). Can you maybe track it through the reports the library sends?

@nabla-c0d3
Copy link
Member

Released as v1.7.0.

OS-ricardomoreirasilva added a commit to OutSystems/TrustKit that referenced this issue Oct 28, 2024
* Fix iOS version; fixes datatheorem#181

* Fix 1 bug, and several static analysis warnings
If shouldExcludeSubdomain is explicitely set to NO, it was treated the
same as YES

* Pod update Demo App to TrustKit 1.6

* Add default circle ci 2.0 file

* Fix CI

* Fix CI

* Fix CI

* Switch to bitrise

* Bump version number

* Update Podfile in demo app to 1.6.1

* Fix test for testPinningValidationSucceeded

This test had an outdated pinset which caused it to fail.
Updated the pinset to include a pin from the current intermediate
CA certificate (Let's Encrypt Authority X3)

* Fix domain selection in overlapping pinsets

Observed behavior of TrustKit showed that, when a domain did not
have an exact match for a pinset, the first matching pinset config
with the IncludeSubdomains flag was selected. This led to
unpredictable behavior because of the nature of iterating through
dictionary keys. The change in this commit modifies TrustKit's
selection algorithm to iterate through all pinset configs and then
select the one that is the closest match (e.g, longest domain).
This matches the industry best practice set by Google in their
native Android pinning implementation, and brings TrustKit's
behavior in line with that of TrustKit-Android.

* [datatheorem#201] Fix pinning configuration in test apps

* Adding DEFINES_MODULE Flags

Currently TrustKit can't be packaged in a static Swift Library without modules.
"Pod package" fail if TrustKit is in other podspec dependency.
Addind this line will generate module maps for swift dependencies.

* Bump version number

* Re-generate documentation

* Update TrustKit in demo app

* Save & load the SPKI disk cache using secure coding.

* Don't rely on external variable for memory allocation

* Replace static allocation with a runtime check

* Re-generate test certificates and simplify tests

* Log decoding error

* Fix remaining tests

* Add script for generating test certificates

* Fix secure coding deserialization

* Fix secure coding deserialization on non-iOS platforms

* Xcode recommended fixes

* Xcode recommended fixes

* Xcode recommended settings

* Bump version number

* Update TrustKit in demo app

* Added SPM support

* Fixed CocoaPods support

* Fixed tests

* Fixed issue with path to TrustKit.h

* Fix location of TrustKit.h for CocoaPods; fixes datatheorem#216

* Bump version number

* Update demo app

* Remove outdated link

* Fix getting started completionHandler unnecessary key

* Fixed SPM support

Signed-off-by: Mohammad Porooshani <[email protected]>

* Fixed Tests and pod support

Signed-off-by: Mohammad Porooshani <[email protected]>

* Bump version number

* Update demo app

* Fix framework to package /Modules files when built

Public headers were added to the xcodeproj file in a way that
a modulemap was no longer being generated, and therefore no
swiftmodule (or swiftinterface) file could be created. This adds
them as explicit public headers instead of a folder copy.

* [datatheorem#210] Fix crash when passing a TLD to check config

* [datatheorem#211] Do not crash on an unsupported key

* [datatheorem#232] Remove non-secure NSKeyedArchiver code

* Fix build warning

* Bump version number

* Update Demo app

* Remove extra spaces

* Add Swift Package Manager to the installation instructions

* [datatheorem#234] Expost static and dynamic Swift packages

* Update README.md

Clarify sample config

* Added a nil check for the value returned from SecCertificateCopySubjectSummary before logging and releasing the value

* Added log for when the certificate subject could not be parsed.

* Added error checking when copying the public key from the certificate.

* Update pinned certs in unit tests

* Update OCMock framework

* Update project settings

* Update CocoaPods

* Bump version number

* Fix pins in demo apps

* Update demo apps settings

* Update demo apps TrustKit version

* Use NS_BLOCK_ASSERTIONS for SwiftPM release builds

Xcode doesn't automatically set the NS_BLOCK_ASSERTIONS flag for SwiftPM release builds.

Use cSettings to set the flag, so NSAssert doesn't crash release builds and behavior is similar to using Carthage or Cocoapods.

See https://forums.swift.org/t/assertions-in-swift-packages/42692 for more info.

* Bump version number

* Update Demo app to use last version of TrustKit

* Update Demo app Xcode settings

* fix deprecation warnings

Fixes deprecation warnings for

  - SecTrustEvaluate
  - SecTrustGetCertificateAtIndex
  - SecTrustCopyPublicKey

* add header/source references for pinning_utils

* remove deprecated references to +[NSURLSession new]

* use dlsym and ifdefs for SecTrustCopyCertificateChain on old SDKs

* tvos, watchos min versions

* check for null error in evaluateTrust

* remove SecEvaluateTrust, bump min OS versions in readme

* remove SecTrustCopyPublicKey, bump min OS versions

* refactor SecTrustEvaluateWithError, bump OS versions, update readme

* evaluate status instead of trustResult of TSKSPKIHashCache

* bump version, update podspec, update Xcode demo app

* update OS versions, swift tools version in SPM package

* enable multipath service type handover on iOS for all NSURLSessions

* avoid using IDFV on iOS and tvOS

* restore iOS 12, tvOS 12, watchOS 4, macOS 10.13 support

* remove easily-misinterpreted bool return value

* restore earlier OS versions in package manager

* fix pod lib lint warnings

* Fix for Trustkit not building on Xcode 14.3 datatheorem#298

* lower deployment versions, fix misc warnings

* Fix for crash reported on iOS 17

* Load library from complete path if loading from default path failed

* use full path for security framework

* bump version to 3.0.3

* Add Privacy Manifest

Fixes datatheorem#319

* Add missing entries to PrivacyInfo.xcprivacy

- Swap Cloudflare domain with Data Theorem for TSKEndToEndSwizzlingTests
- Add PrivacyInfo.xcprivacy to Xcode project for all targets
- Remove noop assign

* Bump version to 3.0.4

* chore: update podspec

* chore: update framework name

This is the name to use when we need to import the library as a xcframework, instead of a CocoaPod.

* chore: add CHANGELOG entry

References: https://outsystemsrd.atlassian.net/browse/RMET-3403

---------

Signed-off-by: Mohammad Porooshani <[email protected]>
Co-authored-by: Alban Diquet <[email protected]>
Co-authored-by: Adam Kaplan <[email protected]>
Co-authored-by: Alban Diquet <[email protected]>
Co-authored-by: Joe Portner <[email protected]>
Co-authored-by: AbbyM <[email protected]>
Co-authored-by: Adam Kaplan <[email protected]>
Co-authored-by: Peter Gammelgaard Poulsen <[email protected]>
Co-authored-by: luancurti <[email protected]>
Co-authored-by: Mohammad Porooshani <[email protected]>
Co-authored-by: David Harris <[email protected]>
Co-authored-by: Craig Siemens <[email protected]>
Co-authored-by: Ethan Arbuckle <[email protected]>
Co-authored-by: Alban Diquet <[email protected]>
Co-authored-by: Eric Chamberlain <[email protected]>
Co-authored-by: Amos Joshua <[email protected]>
Co-authored-by: pawisoon <[email protected]>
Co-authored-by: Darsan-G <[email protected]>
Co-authored-by: aj-dt <[email protected]>
Co-authored-by: uroboro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants