some crashes on using DSA keys

[Simon Josefsson]

Simon Josefsson <simon at josefsson.org> writes:

> There are certainly double-free's going on in pk-libgcrypt.c, and it
> would be useful to see which can be exploited.  I'm looking at the code
> now.

Ok, reading gnutls 2.6.5 code for the functions with 'cleanup' goto's,
this appears to hold:

1) _wrap_gcry_pk_encrypt: double-_gnutls_mpi_release of 'data' on errors
   from gcry_pk_encrypt, gcry_sexp_find_token, gcry_sexp_nth_mpi, or
   _gnutls_mpi_dprint_size.

2) _wrap_gcry_pk_decrypt: double-_gnutls_mpi_release of 'data' on errors
   from gcry_pk_decrypt, gcry_sexp_nth_mpi, or _gnutls_mpi_dprint_size.

3) _wrap_gcry_pk_decrypt: double-gcry_sexp_release of 's_plain',
   's_data' and 's_pkey' on errors from gcry_sexp_nth_mpi, or
   _gnutls_mpi_dprint_size.

4) _wrap_gcry_pk_sign: double-_gnutls_mpi_release of 'hash' always?

5) _wrap_gcry_pk_verify: double-gcry_sexp_release of 's_sig', 's_hash',
   's_pkey' on failures from gcry_pk_verify.

The 4) issue suggests that double-calling of _gnutls_mpi_release isn't a
problem.  Indeed, check that function, it protects against double frees.
So 1), 2), and 4) are never a problem, but they make the code harder to
read.

Then 3) and 6) remains. 6) is clearly possible to trigger, just provide
data that doesn't verify correctly.  This was the original problem.

For 3) it is only possible to trigger if A) the output from
gcry_pk_decrypt doesn't return an mpi on success, or B) if
_gnutls_mpi_dprint_size fails.  The A) would indicate a libgcrypt bug
and B) only appears possible on out-of-memory conditions.  I don't think
these can be exploited, but the code should be fixed anyway.

The code in 6) is used when verifying DSA signatures in GnuTLS.  It's
not used for RSA since TLS doesn't use PKCS#1 padding which libgcrypt
uses.  The function is also used by the OpenCDK library to verify
RSA/DSA signatures.

So it appears possible to cause a double-free to happen, which typically
leads to a DoS, on any GnuTLS application that verifies DSA signatures
during TLS, or uses OpenPGP authentication and verifies RSA/DSA
signatures, by sending an invalid signature.

The minimal patch to fix this problem appears to be:

--- pk-libgcrypt.c-	2009-04-23 10:59:06.000000000 +0200
+++ pk-libgcrypt.c	2009-04-23 10:59:09.000000000 +0200
@@ -511,6 +511,7 @@
   gcry_sexp_release (s_sig);
   gcry_sexp_release (s_hash);
   gcry_sexp_release (s_pkey);
+  s_hash = s_pkey = s_sig = NULL;
 
   if (rc != 0)
     {

/Simon