Memory leaks in oqs ecdh path

[torben-hansen]

I see two memory leaks present in the current code. To setup reproduction:

1. clone oqs openssh repo
2. ./oqs-scripts/clone_liboqs.sh
3. ./oqs-scripts/build_liboqs.sh
4. ./oqs-scripts/build_openssh.sh (requires openssl 3.x in the usual place)
5. oqs-test/run_tests.sh (populates /regress repo with useful things)

Modify sshd_config to listen on 22222. Start server:

$(pwd)/sshd -ddd -h $(pwd)/regress/host.ecdsa-sha2-nistp256 -f sshd_config

Leak 1

kex_ecdh_dec_key_group allocates new memory and saves a reference in *shared_secretp. The function is called twice:

• https://github.com/open-quantum-safe/openssh/blob/OQS-v8/kexoqsecdh.c#L254, and
• https://github.com/open-quantum-safe/openssh/blob/OQS-v8/kexoqsecdh.c#L329

sshbuf_putb does not free ecdh_shared_secret, even if fully consumed. Hence, memory leaks at function exit.

Trace with valgrind

valgrind --trace-children=yes --leak-check=full ./ssh -i regress/host.ecdsa-sha2-nistp256 localhost -p 22222 -oKexAlgorithms=ecdh-nistp256-kyber-512r3-sha256-d00@openquantumsafe.org

[...]

==639219== 328 (72 direct, 256 indirect) bytes in 1 blocks are definitely lost in loss record 885 of 983                    
==639219==    at 0x4848464: calloc (vg_replace_malloc.c:1328)                                                               
==639219==    by 0x142B50: sshbuf_new (sshbuf.c:73)                                                                         
==639219==    by 0x1B6968: kex_ecdh_dec_key_group.constprop.0 (kexoqsecdh.c:57)                                             
==639219==    by 0x1B6D53: kex_kem_generic_with_ec_dec (kexoqsecdh.c:329)                                                   
==639219==    by 0x1B82FC: kex_kem_kyber_512_ecdh_nistp256_dec (kexoqsecdh.c:638)                                           
==639219==    by 0x1A1EDE: input_kex_gen_reply (kexgen.c:440)                                                               
==639219==    by 0x1815A5: ssh_dispatch_run (dispatch.c:113)                                                                
==639219==    by 0x181708: ssh_dispatch_run_fatal (dispatch.c:133)
==639219==    by 0x136CA2: ssh_kex2 (sshconnect2.c:346)
==639219==    by 0x131F7D: ssh_login (sshconnect.c:1565)
==639219==    by 0x116C89: main (ssh.c:1680)

Leak 2

EC_KEY_new_by_curve_name returns a pointer to newly allocated memory, referenced by ecdh_client_key. On the error path, ownership of the memory referenced by ecdh_client_key is not transferred to the caller kex and therefore leaks when exiting the function.

Impose deliberate error to trace error path

diff --git a/kexoqsecdh.c b/kexoqsecdh.c
index 36eabce5..d3447052 100644
--- a/kexoqsecdh.c
+++ b/kexoqsecdh.c
@@ -138,7 +138,7 @@ static int kex_kem_generic_with_ec_keypair(OQS_KEM *kem, struct kex *kex)
     r = SSH_ERR_ALLOC_FAIL;
     goto out;
   }
-  if (EC_KEY_generate_key(ecdh_client_key) != 1) {
+  if (1 || EC_KEY_generate_key(ecdh_client_key) != 1) {
     r = SSH_ERR_LIBCRYPTO_ERROR;
     goto out;
   }

Trace with valgrind

valgrind --trace-children=yes --leak-check=full ./ssh -i regress/host.ecdsa-sha2-nistp256 localhost -p 22222 -oKexAlgorithms=ecdh-nistp256-kyber-512r3-sha256-d00@openquantumsafe.org

[...]

==639585== 1,316 (112 direct, 1,204 indirect) bytes in 1 blocks are definitely lost in loss record 856 of 893
==639585==    at 0x484386F: malloc (vg_replace_malloc.c:381)
==639585==    by 0x4A1116D: CRYPTO_zalloc (in /usr/lib64/libcrypto.so.3.0.8)
==639585==    by 0x499DD1D: ??? (in /usr/lib64/libcrypto.so.3.0.8)
==639585==    by 0x499F27C: EC_KEY_new_by_curve_name_ex (in /usr/lib64/libcrypto.so.3.0.8)
==639585==    by 0x1B6866: kex_kem_generic_with_ec_keypair (kexoqsecdh.c:137)
==639585==    by 0x1B809C: kex_kem_kyber_512_ecdh_nistp256_keypair (kexoqsecdh.c:611)
==639585==    by 0x1A24E7: kex_gen_client (kexgen.c:218)
==639585==    by 0x19E2FB: kex_input_kexinit (kex.c:698)
==639585==    by 0x1815A5: ssh_dispatch_run (dispatch.c:113)
==639585==    by 0x181708: ssh_dispatch_run_fatal (dispatch.c:133)
==639585==    by 0x136CA2: ssh_kex2 (sshconnect2.c:346)
==639585==    by 0x131F7D: ssh_login (sshconnect.c:1565)