search

apple / swift-crypto

Open-source implementation of a substantial portion of the API of Apple CryptoKit suitable for use on Linux platforms.
https://apple.github.io/swift-crypto
Apache License 2.0
1.45k stars 154 forks source link

Possible Over-Iteration in PrettyBytes #139

Open VaslD opened 1 year ago

VaslD commented 1 year ago

I was looking for a not %02x way of converting binary to hex String, and I stumbled upon the implementation in PrettyBytes.swift.

https://github.com/apple/swift-crypto/blob/9cc89f0170308b813af05dadcd26f9a2dee47713/Sources/Crypto/Util/PrettyBytes.swift#L45-L51

I'm not 100% sure about the Sequence conformance of non-contiguous DataProtocol, but it seemed to me that the self on line 46 should be the ignored parameter $0 (or maybe eliminate the region logic since looping over self most likely would have taken non-contiguous memory regions into account?), otherwise the code reads:

For each segment of the data buffer, loop over the entirety of data buffer and convert each byte to two hex chars.

Unless non-contiguous DataProtocol has non-stable indices and iterators, this code will over/re-iterate the data buffer.

Lukasa commented 1 year ago

Yup, it sure does! Would you like to submit a patch to fix it? As a practical matter I think we can ignore the regions here, and just use the sequence conformance directly.

VaslD commented 1 year ago

PR done. Don't know how to construct a multi-region Data to test it yet, but it shouldn't get worse. :-)