From d082a5c7fcb9114c4c1e515af4be0bce67843442 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 23 May 2024 20:41:46 +0200 Subject: [PATCH] Fix memory leaks in ext/sodium on failure of some functions Infallible in practice right now, but should be fixed as infallible today does not mean infallible tomorrow: - sodium_crypto_sign_publickey_from_secretkey - sodium_crypto_kx_seed_keypair - sodium_crypto_kx_keypair - sodium_crypto_auth - sodium_crypto_sign_ed25519_sk_to_curve25519 - sodium_pad Fallible today: - sodium_crypto_sign_ed25519_pk_to_curve25519 --- ext/sodium/libsodium.c | 7 +++++++ ...gn_ed25519_pk_to_curve25519_failure_leak.phpt | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 ext/sodium/tests/sodium_crypto_sign_ed25519_pk_to_curve25519_failure_leak.phpt diff --git a/ext/sodium/libsodium.c b/ext/sodium/libsodium.c index 944ef28fc3a43..a50328d929879 100644 --- a/ext/sodium/libsodium.c +++ b/ext/sodium/libsodium.c @@ -992,6 +992,7 @@ PHP_FUNCTION(sodium_crypto_sign_publickey_from_secretkey) if (crypto_sign_ed25519_sk_to_pk((unsigned char *) ZSTR_VAL(publickey), (const unsigned char *) secretkey) != 0) { + zend_string_efree(publickey); zend_throw_exception(sodium_exception_ce, "internal error", 0); RETURN_THROWS(); @@ -2475,6 +2476,7 @@ PHP_FUNCTION(sodium_crypto_kx_seed_keypair) crypto_generichash(sk, crypto_kx_SECRETKEYBYTES, seed, crypto_kx_SEEDBYTES, NULL, 0); if (crypto_scalarmult_base(pk, sk) != 0) { + zend_string_efree(keypair); zend_throw_exception(sodium_exception_ce, "internal error", 0); RETURN_THROWS(); } @@ -2496,6 +2498,7 @@ PHP_FUNCTION(sodium_crypto_kx_keypair) pk = sk + crypto_kx_SECRETKEYBYTES; randombytes_buf(sk, crypto_kx_SECRETKEYBYTES); if (crypto_scalarmult_base(pk, sk) != 0) { + zend_string_efree(keypair); zend_throw_exception(sodium_exception_ce, "internal error", 0); RETURN_THROWS(); } @@ -2672,6 +2675,7 @@ PHP_FUNCTION(sodium_crypto_auth) if (crypto_auth((unsigned char *) ZSTR_VAL(mac), (const unsigned char *) msg, msg_len, (const unsigned char *) key) != 0) { + zend_string_efree(mac); zend_throw_exception(sodium_exception_ce, "internal error", 0); RETURN_THROWS(); } @@ -2731,6 +2735,7 @@ PHP_FUNCTION(sodium_crypto_sign_ed25519_sk_to_curve25519) if (crypto_sign_ed25519_sk_to_curve25519((unsigned char *) ZSTR_VAL(ecdhkey), (const unsigned char *) eddsakey) != 0) { + zend_string_efree(ecdhkey); zend_throw_exception(sodium_exception_ce, "conversion failed", 0); RETURN_THROWS(); } @@ -2758,6 +2763,7 @@ PHP_FUNCTION(sodium_crypto_sign_ed25519_pk_to_curve25519) if (crypto_sign_ed25519_pk_to_curve25519((unsigned char *) ZSTR_VAL(ecdhkey), (const unsigned char *) eddsakey) != 0) { + zend_string_efree(ecdhkey); zend_throw_exception(sodium_exception_ce, "conversion failed", 0); RETURN_THROWS(); } @@ -3036,6 +3042,7 @@ PHP_FUNCTION(sodium_pad) #if SODIUM_LIBRARY_VERSION_MAJOR > 9 || (SODIUM_LIBRARY_VERSION_MAJOR == 9 && SODIUM_LIBRARY_VERSION_MINOR >= 6) if (sodium_pad(NULL, (unsigned char *) ZSTR_VAL(padded), unpadded_len, (size_t) blocksize, xpadded_len + 1U) != 0) { + zend_string_efree(padded); zend_throw_exception(sodium_exception_ce, "internal error", 0); RETURN_THROWS(); } diff --git a/ext/sodium/tests/sodium_crypto_sign_ed25519_pk_to_curve25519_failure_leak.phpt b/ext/sodium/tests/sodium_crypto_sign_ed25519_pk_to_curve25519_failure_leak.phpt new file mode 100644 index 0000000000000..1e4e74eecbe03 --- /dev/null +++ b/ext/sodium/tests/sodium_crypto_sign_ed25519_pk_to_curve25519_failure_leak.phpt @@ -0,0 +1,16 @@ +--TEST-- +Memory leak on sodium_crypto_sign_ed25519_pk_to_curve25519() failure +--EXTENSIONS-- +sodium +--FILE-- +getMessage(); +} + +?> +--EXPECT-- +conversion failed