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

using deprecated in iOS13 MD5 for file cache keys #582

Open
bolsinga opened this issue Nov 4, 2020 · 4 comments
Open

using deprecated in iOS13 MD5 for file cache keys #582

bolsinga opened this issue Nov 4, 2020 · 4 comments

Comments

@bolsinga
Copy link
Contributor

bolsinga commented Nov 4, 2020

Added with #189. It is not clear to me from reading that what the issue was, but using this will get deprecation warnings when targeting iOS13.

@dyomin-d
Copy link

I face the same security issue. Here you may find the message from Data Theorem:

The vulnerable code locations use the CC_MD hashing functions, which leverage hashing algorithms (including MD2 and MD5) that are proven to be vulnerable to collision attacks, and are unsuitable for modern use.

Apple has officially deprecated these APIs in the iOS 13.0 SDK. They state in the CommonCrypto headers:

"These functions are cryptographically broken and should not be used in security contexts. Clients should migrate to SHA256 (or stronger)."

The following code locations within the App use the deprecated CC_MD functions:
-[PINRemoteImageManager cacheKeyForURL:processorKey:resume:] calls _CC_MD5_Init()
-[PINRemoteImageManager cacheKeyForURL:processorKey:resume:] calls _CC_MD5_Final()
etc.

Please prioritise the issue as it is a serious security downlift.

@SAGESSE-CN
Copy link
Contributor

SAGESSE-CN commented Jun 1, 2021

Please prioritise the issue as it is a serious security downlift.

This will not cause any security issues.

The vulnerable code locations use the CC_MD hashing functions, which leverage hashing algorithms (including MD2 and MD5) that are proven to be vulnerable to collision attacks, and are unsuitable for modern use.

When you use the MD2/MD5 in crypto system world that's true, but we don't, it's not a problem just a warning. cacheKeyForURL does not use MD5 for any encryption.

We do need to do some data digesting of the URL to prevent the name too long.

Suggestion:

  1. Continue with CC_MD5, turn off the warnings before call, since we already know why
  2. Replace with the new Hash algorithm

@garrettmoon
Copy link
Collaborator

@SAGESSE-CN is correct, this probably isn't much of an attack vector (I suppose your app could be tricked into showing an item from the cache when it wasn't supposed to…)

That said, it's deprecated, we should move to a separate hashing algorithm. Sadly this will mean either we'll need to add support for migrate caches for folks so it won't be easy to address.

@dyomin-d
Copy link

dyomin-d commented Jun 4, 2021

Thank you for the comment. I hope you will put it in your roadmap as it influences consumer's app security.

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

No branches or pull requests

4 participants