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

Fix memleak in EdDSA key conversion API #6853

Open
maxtropets opened this issue Feb 21, 2025 · 4 comments
Open

Fix memleak in EdDSA key conversion API #6853

maxtropets opened this issue Feb 21, 2025 · 4 comments
Assignees

Comments

@maxtropets
Copy link
Collaborator

Minimal repro

  auto jwk_str =
    "{\"crv\":\"X25519\",\"d\":\"iBajS4-xI2dTNNY_WZ94kX8EfVUy_oNMGDmnnA92IlA\","
    "\"kid\":\"4t4Ravodr_J_FHqBzPcy3ZW0PcwB_31AtbV4fyOOOwc\",\"kty\":\"OKP\","
    "\"x\":\"YmDs4SXGtFbHE5Ad4dw6qjoDVCrQBOOIshA9wN55Az4\"}";
  JsonWebKeyEdDSAPrivate jwk = nlohmann::json::parse(jwk_str);
  auto kp2 = ccf::crypto::make_eddsa_key_pair(jwk);

causes

==5128==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x58fe3003a1bf in malloc (/home/maxtropets/workspace/CCF/build/crypto_test+0x2ff1bf)
    #1 0x7c54ac218c30 in CRYPTO_malloc (/usr/lib/libcrypto.so.3+0x218c30)

SUMMARY: AddressSanitizer: 56 byte(s) leaked in 1 allocation(s).
@maxtropets maxtropets self-assigned this Feb 21, 2025
@maxtropets
Copy link
Collaborator Author

Happens in

  EdDSAKeyPair_OpenSSL::EdDSAKeyPair_OpenSSL(const JsonWebKeyEdDSAPrivate& jwk)
  {
    OpenSSL::CHECKNULL(
      key = EVP_PKEY_new_raw_private_key(
        curve, nullptr, d_raw.data(), d_raw.size()));
  }

@maxtropets
Copy link
Collaborator Author

maxtropets commented Feb 21, 2025

Basic googling didn't reveal any known bugs. Tried quick possible quick mitigations

  • slapping OPENSSL_cleanup()
  • using EVP_PKEY_new_raw_private_key_ex()

No luck here.

Didn't find any other examples of how does one create an EdDSA key.

Will try build OpenSSL from source and get a more precise leak location.

@maxtropets maxtropets changed the title Fix memleak in EdDSA key convtion API Fix memleak in EdDSA key convertion API Feb 21, 2025
@achamayou achamayou changed the title Fix memleak in EdDSA key convertion API Fix memleak in EdDSA key conversion API Feb 21, 2025
@maxtropets
Copy link
Collaborator Author

Looks similar openssl/openssl#11064

@maxtropets
Copy link
Collaborator Author

Tried

root [ /workspace/build ]# ./crypto_test
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases: 1 | 1 passed | 0 failed | 0 skipped
[doctest] assertions: 0 | 0 passed | 0 failed |
[doctest] Status: SUCCESS!

=================================================================
==30066==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x558bc60961bf in malloc (/workspace/build/crypto_test+0x2ff1bf)
    #1 0x7f7ffe204c30 in CRYPTO_malloc (/usr/lib/libcrypto.so.3+0x218c30)

SUMMARY: AddressSanitizer: 56 byte(s) leaked in 1 allocation(s).
root [ /workspace/build ]# cd /usr/l
bash: cd: /usr/l: No such file or directory
root [ /workspace/build ]# cd /usr/lib
root [ /usr/lib ]# cd /usr/lc^C
root [ /usr/lib ]# ^C
root [ /usr/lib ]# rm libcrypto.so.3
root [ /usr/lib ]# ln -s /opt/openssl/lib64/libcrypto.so.3 libcrypto.so.3
root [ /usr/lib ]# ln -s /opt/openssl/lib64/libcrypto.so.3 libcrypto.s^C3
root [ /usr/lib ]# rm libssl.so.3
root [ /usr/lib ]# ln -s /opt/openssl/lib64/libssl.so.3 libssl.so.3
root [ /usr/lib ]# cd -
/workspace/build
root [ /workspace/build ]# ./crypto_test
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases: 1 | 1 passed | 0 failed | 0 skipped
[doctest] assertions: 0 | 0 passed | 0 failed |
[doctest] Status: SUCCESS!
root [ /workspace/build ]#

Long story short, rebulding openssl 3.3.2 from source solved the issue. I assume there's an issue with symcrypt here.

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

1 participant