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

Detect Invalid Initialization Vector Size. #26

Closed
iosdevzone opened this issue Nov 6, 2017 · 4 comments
Closed

Detect Invalid Initialization Vector Size. #26

iosdevzone opened this issue Nov 6, 2017 · 4 comments

Comments

@iosdevzone
Copy link

Since this code is based on IDZSwiftCommonCrypto it has inherited some of that library's flaws. While the library currently pays quite a bit of attention to keys, it has no error checking on IV size.

If a user supplies too small an IV, some uninitialized bytes will used by CommonCrypto. This is not a security risk, it just leads to incorrect results and difficult to track down bugs.

We're adding additional error checking to try to catch these problems before they occur.

You may want to take a look at iosdevzone/IDZSwiftCommonCrypto#79 to see the changes we're making!

@iosdevzone iosdevzone changed the title Detect Invalid Initialization Vector size. Detect Invalid Initialization Vector Size. Nov 6, 2017
@billabt
Copy link
Collaborator

billabt commented Nov 7, 2017

Thanks for the heads up. I'll incorporate the enhanced error checking.

@billabt billabt closed this as completed Nov 7, 2017
billabt pushed a commit that referenced this issue Nov 7, 2017
@iosdevzone
Copy link
Author

iosdevzone commented Nov 7, 2017

You're welcome!

The change you made near line 643 of StreamCryptor.swift is not correct. It should be:

    iv.utf8.count

iv.count will give you the number of Characters not bytes.

@billabt
Copy link
Collaborator

billabt commented Nov 7, 2017

Thanks for the catch. Unfortunately, I’ve got a nasty habit of making that particular mistake. Thanks again for catching it.

@iosdevzone
Copy link
Author

Happens to us all! Actually this made me aware of the fact that we're relying on some possibly undocumented behavior in how String is passed as an UnsafeRawPointer. Will be checking it out and let you know my conclusion. This route was to avoid the possible performance hit of explicitly converting to [UInt8]. I'll open a separate issue/PR when appropriate.

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

2 participants