Skip to content

[MFA-2038] Unsafe code removal. #112

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

Merged
merged 1 commit into from
Apr 7, 2023
Merged

[MFA-2038] Unsafe code removal. #112

merged 1 commit into from
Apr 7, 2023

Conversation

Artelas
Copy link
Contributor

@Artelas Artelas commented Apr 4, 2023

Related to
#105
Removed unsafe code. Adjusted solution to match unit tests by excluding excessive checks.

// digestData.count - 1 will always be >0, because digestData depends on algorythm and can be only 20, 32, or 64.
let offset = Int(digestData[digestData.count - 1] & 0x0f)
// offset is always <=15, length is always 4, so prefix will always be within digestData count.
var hash = digestData.dropFirst(offset).prefix(length).reduce(0, { $0 << 8 | UInt32($1) })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding correctly, what the original code did (from L85 to L89) was roughly the following:

Screenshot 2023-04-04 at 8 06 25 PM

Could you please expand how reducing the data (after excluding the offset) by bit-shifting to the left is equivalent to pointee of start.withMemoryRebound(to: UInt32.self, capacity: 1) { $0 }?

Copy link
Contributor Author

@Artelas Artelas Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let value = start.withMemoryRebound(to: UInt32.self, capacity: 1) { $0 }

Here we have start as pointer to array of UInt8. withMemoryRebound interpret this array as single UInt32 number. For example for array of 4 UInt8 [0x12, 0x34, 0x56, 0x78], withMemoryRebound will produce single UInt32: 0x78563412. Which is then converted by constructor UInt32(bigEndian:) to 0x1234567.

.reduce(0, { $0 << 8 | UInt32($1) })

Here on the first iteration we have UInt32 equal to 0. Which is 0x00000000. First we shift it by 8 bytex, which produces the same value and doing OR with first element which is 0x12. resulting in 0x00000012. On the next iteration we shift it by 8 bytes resulting in 0x00001200. After doing OR with the next element 0x34, the result is 0x00001234. By the end of iterations we have 0x12345678, which is equal to result of withMemoryRebound with bigEndian.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks for the clarification!

Copy link
Contributor

@ionutmanolache-okta ionutmanolache-okta Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shift it by 8 bytes
I assume you meant 8 bits

@ionutmanolache-okta
Copy link
Contributor

@Artelas,
#105 (comment)
Was it producing a fatal error?
I built this many times for iOS 16, and it did not produce that error.

@Artelas
Copy link
Contributor Author

Artelas commented Apr 7, 2023

@ionutmanolache-okta. Yes, every time I've tried to run tests on iOS 16, I got runtime fatal error.

@Artelas Artelas merged commit 2396dea into master Apr 7, 2023
@ObidosDev
Copy link

Greate! When release of new version? :)

@diegose diegose deleted the ab-mfa-2038 branch July 31, 2023 15:11
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

Successfully merging this pull request may close these issues.

4 participants