Switch from cryptonite library to crypton and fix cbits #95
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erikd
commented
Oct 11, 2024
| @@ -253,9 +253,9 @@ static void serialize_index32(uint8_t *out, uint32_t index, derivation_scheme_mo | |||
|
|
|||
| static void add_left(ed25519_secret_key res_key, uint8_t *z, ed25519_secret_key priv_key, derivation_scheme_mode mode) | |||
| { | |||
| uint8_t zl8[32]; | |||
| ed25519_secret_key zl8; | |||
There was a problem hiding this comment.
The unit8_t array had 32 elements (which is what caused the error message) and is replace with ed25519_secret_key which is 64 elements.
There was a problem hiding this comment.
Before the fix, the error was:
cbits/encrypted_sign.c:304:9: error:
warning: ‘cardano_crypto_ed25519_publickey’ reading 64 bytes from a region of size 32 [-Wstringop-overread]
304 | cardano_crypto_ed25519_publickey(zl8, pub_zl8);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
304 | cardano_crypto_ed25519_publickey(zl8, pub_zl8);
|
erikd
commented
Oct 11, 2024
|
|
||
| memset(zl8, 0, 32); | ||
| memset(zl8, 0, 64); |
There was a problem hiding this comment.
Because the size of the z18 array has increased to 64 we set all 64 elements to zero.
erikd
force-pushed
the
erikd/crypton
branch
2 times, most recently
from
69286e4
to
e85789b
Compare
angerman
approved these changes
Oct 11, 2024
perturbing
approved these changes
Oct 14, 2024
There was a problem hiding this comment.
LGTM,
I am a bit puzzled why this compiler did not catch this buffer mismatch, as this was always trying to read 64 bytes.
6 tasks
|
Darwin 8.10.7 is missing llvm 🤦 |
The cryptonite library is deprecated and has been replaced by crypton which is a drop in replacement. For the `cbits` code, replace all instances of `cryptonite` with `crypton`.
Pretty sure this warning is only generated with GCC versions >= 14.2.
A type and a function were defined as follows:
typedef unsigned char ed25519_secret_key[64];
void cardano_crypto_ed25519_publickey(const ed25519_secret_key sk, ed25519_public_key pk);
and then the code had:
uint8_t zl8[32];
cardano_crypto_ed25519_publickey(zl8, pub_zl8);
The compiler error was squashed by changing the type of `uint8_t [32]` to
`ed25519_secret_key` which is 64 bytes in size and then updating the
`memset` call to zero the whole array.
erikd
force-pushed
the
erikd/crypton
branch
2 times, most recently
from
8746e56
to
2607d9f
Compare
- Use haskell-actions/setup instead of haskell/actions/setup - Bump from v1 to v2 - Don't pin the cabal-version. Haskell-actions already bring in a newer version.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The cryptonite library is deprecated and has been replaced by crypton which is a drop in replacement.
cbits: Squash a Wstringop-overread error (This needs very careful review).