[gnutls-dev] more than one trusted certificate - buffer overflow

[Max Kellermann]

Hi,

I'm getting bus errors when I try to add more than one certificate
with gnutls_certificate_set_x509_trust().  Backtrace:

#0  _gnutls_x509_crt_get_raw_dn2 (cert=0x4449657571696e,
 whom=0x2aaaaac2a127 "issuer", start=0x7fffffe93940) at x509.c:1148
#1  0x00002aaaaabfe773 in generate_rdn_seq (res=0x53ac20) at
 gnutls_x509.c:1030
#2  0x00002aaaaabfed3d in gnutls_certificate_set_x509_trust
 (res=0x53ac20, ca_list=0x7fffffe93d60, ca_list_size=2)
    at gnutls_x509.c:1344
#3  0x000000000040371c in reload_credentials (instance=0x7fffffe93d20)
 at server/wrapper.c:458
[...]

I'm using libgnutls12 1.2.9-1 (Debian) on AMD64.  CVS HEAD still
carries this bug.

I found strange code in this function, see gnutls_x509.c lines 1312+:

  for (i = 0; i < ca_list_size; i++) {
      ret = gnutls_x509_crt_init(&res->x509_ca_list[i +
                                 res->x509_ncas]);
      [..]
      res->x509_ncas++;
  }

Both "i" and "res->x509_ncas" are incremented in every iteration, that
means, only even indices are being initialized - and it will overflow
the "res->x509_ca_list" buffer.

The same bug is present in gnutls_certificate_set_x509_crl().

Valgrind confirmed my theory:

Invalid write of size 8
   at 0x4B69C3F: gnutls_x509_crt_init (x509.c:54)
   by 0x4B5ACF4: gnutls_certificate_set_x509_trust (gnutls_x509.c:1328)
   by 0x40371B: reload_credentials (wrapper.c:458)
   by 0x40392C: init_tls (wrapper.c:523)
   by 0x404D25: main (wrapper.c:1118)

Solution: don't add "i" and "res->x509_ncas", just "res->x509_ncas"
gives us the correct index.  See patch.

You might need a lot of fantasy to imagine a remote exploit for this
buffer overflow, but the fact that this bug exists, shows that nobody
has ever tried to load more than one trusted certificate into
libgnutls...

Max

-------------- next part --------------
--- gnutls12-1.2.9/lib/gnutls_x509.c.orig	2006-02-01 14:24:37.778492250 +0100
+++ gnutls12-1.2.9/lib/gnutls_x509.c	2006-02-01 14:24:59.971879250 +0100
@@ -1325,17 +1325,17 @@
     }
 
     for (i = 0; i < ca_list_size; i++) {
-	ret = gnutls_x509_crt_init(&res->x509_ca_list[i + res->x509_ncas]);
+	ret = gnutls_x509_crt_init(&res->x509_ca_list[res->x509_ncas]);
 	if (ret < 0) {
 	    gnutls_assert();
 	    return ret;
 	}
 
-	ret = _gnutls_x509_crt_cpy(res->x509_ca_list[i + res->x509_ncas],
+	ret = _gnutls_x509_crt_cpy(res->x509_ca_list[res->x509_ncas],
 				   ca_list[i]);
 	if (ret < 0) {
 	    gnutls_assert();
-	    gnutls_x509_crt_deinit(res->x509_ca_list[i + res->x509_ncas]);
+	    gnutls_x509_crt_deinit(res->x509_ca_list[res->x509_ncas]);
 	    return ret;
 	}
 	res->x509_ncas++;
@@ -1615,7 +1615,7 @@
     }
 
     for (i = 0; i < crl_list_size; i++) {
-	ret = _gnutls_x509_crl_cpy(res->x509_crl_list[i + res->x509_ncrls],
+	ret = _gnutls_x509_crl_cpy(res->x509_crl_list[res->x509_ncrls],
 				   crl_list[i]);
 	if (ret < 0) {
 	    gnutls_assert();