Cleanups and several bug fixes found by Tomas Mraz.

"I've patched the following problems in the code found by review of
gnutls-2.8.5 code done by Steve Grubb.

See the patch attached.

The gnutls_constate.c bug might be potentially serious so I've decided
to mail it to you directly, not to the public mailing list.

The auth_cert.c change is just cleanup of the code.

In gnutls_openssl.c I've just fixed the potential crasher, correct fix
would require using asprintf or precomputed length of the buffer to
allocate a memory.

The certtool.c change is again just a cleanup."

diff --git a/lib/auth_cert.c b/lib/auth_cert.c
index 0dbba1f9259c4e777ef14a1ae3b6d78d6aa143ff..5a6d8beae366de4cdf84e27f8d9f6e5f7aed5356 100644 (file)
--- a/lib/auth_cert.c
+++ b/lib/auth_cert.c
@@ -1895,7 +1895,7 @@ _gnutls_server_select_cert (gnutls_session_t session,
                            gnutls_pk_algorithm_t requested_algo)
 {
   unsigned i;
-  int idx, ret;
+  int idx;
   gnutls_certificate_credentials_t cred;
 
   cred = (gnutls_certificate_credentials_t)
@@ -1914,7 +1914,6 @@ _gnutls_server_select_cert (gnutls_session_t session,
 
   /* Otherwise... */
 
-  ret = 0;
   idx = -1;                    /* default is use no certificate */
 
 
@@ -1949,7 +1948,7 @@ _gnutls_server_select_cert (gnutls_session_t session,
   /* store the certificate pointer for future use, in the handshake.
    * (This will allow not calling this callback again.)
    */
-  if (idx >= 0 && ret == 0)
+  if (idx >= 0)
     {
       _gnutls_selected_certs_set (session,
                                  &cred->cert_list[idx][0],
@@ -1958,9 +1957,9 @@ _gnutls_server_select_cert (gnutls_session_t session,
     }
   else
     /* Certificate does not support REQUESTED_ALGO.  */
-    ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS;
+    return GNUTLS_E_INSUFFICIENT_CREDENTIALS;
 
-  return ret;
+  return 0;
 }
 
 /* Frees the rsa_info_st structure.

diff --git a/lib/gnutls_constate.c b/lib/gnutls_constate.c
index 9afd897092deecf9277b1f244e91270a06a554ab..d3fd25646bad3a599d2a0d7df5cfd6858dfe68af 100644 (file)
--- a/lib/gnutls_constate.c
+++ b/lib/gnutls_constate.c
@@ -431,7 +431,7 @@ _gnutls_connection_state_init (gnutls_session_t session)
 
 /* Setup the master secret 
  */
-  if ((ret = _gnutls_generate_master (session, 0), 0) < 0)
+  if ((ret = _gnutls_generate_master (session, 0)) < 0)
     {
       gnutls_assert ();
       return ret;

diff --git a/lib/gnutls_sig.c b/lib/gnutls_sig.c
index af14f6b02929a2570854a5730e0eaa1674e3f1e5..f75a705ac6d3465fdfd99f61d1bd7f5c01a0dc33 100644 (file)
--- a/lib/gnutls_sig.c
+++ b/lib/gnutls_sig.c
@@ -314,7 +314,7 @@ _gnutls_verify_sig (gnutls_cert * cert,
   int ret;
   gnutls_datum_t vdata;
 
-  if (cert->version == 0 || cert == NULL)
+  if (cert == NULL || cert->version == 0)
     {                          /* this is the only way to check
                                 * if it is initialized
                                 */
@@ -324,8 +324,7 @@ _gnutls_verify_sig (gnutls_cert * cert,
 
   /* If the certificate supports signing continue.
    */
-  if (cert != NULL)
-    if (cert->key_usage != 0)
+  if (cert->key_usage != 0)
       if (!(cert->key_usage & KEY_DIGITAL_SIGNATURE))
        {
          gnutls_assert ();

diff --git a/libextra/gnutls_openssl.c b/libextra/gnutls_openssl.c
index 38ae5a86cf63d4a6a2090f058d18d59f19481fc7..fee62f6b49b732b5be17925370882706ecdf40c1 100644 (file)
--- a/libextra/gnutls_openssl.c
+++ b/libextra/gnutls_openssl.c
@@ -887,9 +887,10 @@ X509_get_issuer_name (const X509 * cert)
 char *
 X509_NAME_oneline (gnutls_x509_dn * name, char *buf, int len)
 {
-  memset (buf, 0, len);
+  /* XXX openssl allocates buffer if buf == NULL */
   if (!buf)
     return NULL;
+  memset (buf, 0, len);
 
   snprintf (buf, len - 1,
            "C=%s, ST=%s, L=%s, O=%s, OU=%s, CN=%s/Email=%s",

diff --git a/src/certtool.c b/src/certtool.c
index 145bcebbab142a4e681f5ddcd661517fe61b6532..d41be38b209172cf1e79b78e3619acbfa3f733c0 100644 (file)
--- a/src/certtool.c
+++ b/src/certtool.c
@@ -2176,7 +2176,6 @@ _verify_x509_mem (const void *cert, int cert_size)
   /* Verify using internal algorithm too. */
   {
     int verify_status;
-    int comma;
 
     ret = gnutls_x509_crt_list_verify (x509_cert_list, x509_ncerts,
                                       &x509_cert_list[x509_ncerts - 1], 1,
@@ -2193,28 +2192,22 @@ _verify_x509_mem (const void *cert, int cert_size)
     if (verify_status & GNUTLS_CERT_INVALID)
       {
        fprintf (outfile, "Not verified");
-       comma = 1;
       }
     else
       {
        fprintf (outfile, "Verified");
-       comma = 1;
       }
 
     if (verify_status & GNUTLS_CERT_SIGNER_NOT_CA)
       {
-       if (comma)
-         fprintf (outfile, ", ");
+       fprintf (outfile, ", ");
        fprintf (outfile, "Issuer is not a CA");
-       comma = 1;
       }
 
     if (verify_status & GNUTLS_CERT_INSECURE_ALGORITHM)
       {
-       if (comma)
-         fprintf (outfile, ", ");
+       fprintf (outfile, ", ");
        fprintf (outfile, "Insecure algorithm");
-       comma = 1;
       }
 
     fprintf (outfile, ".\n");

diff --git a/src/cfg/cfg+.c b/src/cfg/cfg+.c
index db019114c6199b553d21d7f9a4728e85a4022bee..1fe611ec6b83c4671b4714ea080a9e78415af3f3 100644 (file)
--- a/src/cfg/cfg+.c
+++ b/src/cfg/cfg+.c
@@ -72,7 +72,7 @@ cfg_get_context(options)
        for (i = 0; i < CFG_N_PROPS; i++) {
                con->prop[i] = PLATON_FUNC(strdyn_create_ar)(cfg_default_properties[i]);
                if (con->prop[i] == NULL) {
-                       /* TODO: possible freeing on failure */
+                       cfg_free_context(con);
                        return NULL;
                }
        }

diff --git a/src/cfg/platon/str/strdyn.c b/src/cfg/platon/str/strdyn.c
index cc57672b595560a6ed0bd00e65649bdeb1d28e1f..34c0247b0d16df5781f491eabe4bb439f6dcb71b 100644 (file)
--- a/src/cfg/platon/str/strdyn.c
+++ b/src/cfg/platon/str/strdyn.c
@@ -316,15 +316,19 @@ PLATON_FUNC(strdyn_explode_str)(str, sep)
 
                s_size = strstr(s, sep) - s;
 
-               if ((ar[i] = (char*) malloc((s_size + 1) * sizeof(char))) == NULL)
+               if ((ar[i] = (char*) malloc((s_size + 1) * sizeof(char))) == NULL) {
+                       PLATON_FUNC(strdyn_free)(ar);
                        return NULL;
+               }
 
                strncpy(ar[i], s, s_size);
                ar[i][s_size] = '\0';
        }
 
-       if ((ar[ar_size] = strdup(s)) == NULL)
+       if ((ar[ar_size] = strdup(s)) == NULL) {
+               PLATON_FUNC(strdyn_free)(ar);
                return NULL;
+       }
 
        ar[ar_size + 1] = NULL;
 

diff --git a/src/serv.c b/src/serv.c
index a8eb8fa513bfa1dc5c13f20966f2c3b84ca4162b..7cee7c3836921312ef9bc1ddb106e070d02e4c61 100644 (file)
--- a/src/serv.c
+++ b/src/serv.c
@@ -500,7 +500,10 @@ peer_print_info (gnutls_session_t session, int *ret_length,
 
   http_buffer = malloc (len);
   if (http_buffer == NULL)
-    return NULL;
+    {
+      free(crtinfo);
+      return NULL;
+    }
 
   strcpy (http_buffer, HTTP_BEGIN);
 
@@ -617,6 +620,7 @@ peer_print_info (gnutls_session_t session, int *ret_length,
       strcat (http_buffer, "<hr><PRE>");
       strcat (http_buffer, crtinfo);
       strcat (http_buffer, "\n</PRE>\n");
+      free(crtinfo);
     }
 
   strcat (http_buffer, "<hr><P>Your HTTP header was:<PRE>");