Re: [PATCH] pki_crypto: Avoid segfault with OpenSSL 1.1.0

[Jakub Jelen <jjelen@xxxxxxxxxx>]

On Tue, 2017-11-07 at 11:53 -0500, Jon Simons wrote:
> On 11/7/17 4:11 AM, Jakub Jelen wrote:
> > Hello,
> > this patch is an addition to the commit 954da14 which is trying to
> > use
> > non-deprecated functions in OpenSSL 1.1.0.
> > 
> > But the newly function needs special allocation of the dsa
> > structure
> > before, which was missing. See the attached patch (or on github
> > [1]).
> > 
> > [1] https://github.com/Jakuje/libssh/commit/dcdba1a
> 
> I believe that if DSA_generate_parameters_ex fails, the key->dsa
> needs to be DSA_free'd and then set to NULL in the error-out path
> on line 469:

Yes, you are right. Thank you for review. The patch resolving this is
attached. It also improves the first condition could be written more
consistently with the rest of the code.

Thanks,
Jakub

From 44032c04f9f539a9269f7e786a7e15c348077e4e Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@xxxxxxxxxx>
Date: Wed, 8 Nov 2017 15:35:08 +0100
Subject: [PATCH] pki_crypto: Avoid potential memory leak

Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx>
---
 src/pki_crypto.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/pki_crypto.c b/src/pki_crypto.c
index 2a8e46ec..e34f197f 100644
--- a/src/pki_crypto.c
+++ b/src/pki_crypto.c
@@ -455,7 +455,7 @@ int pki_key_generate_dss(ssh_key key, int parameter){
     int rc;
 #if OPENSSL_VERSION_NUMBER > 0x10100000L
     key->dsa = DSA_new();
-    if (!key->dsa) {
+    if (key->dsa == NULL) {
         return SSH_ERROR;
     }
     rc = DSA_generate_parameters_ex(key->dsa,
@@ -466,6 +466,8 @@ int pki_key_generate_dss(ssh_key key, int parameter){
                                     NULL,  /* h_ret */
                                     NULL); /* cb */
     if (rc != 1) {
+        DSA_free(key->dsa);
+        key->dsa = NULL;
         return SSH_ERROR;
     }
 #else
-- 
2.13.6