[Openswan dev] PATCH: X.509 Off by one bug in crl processing (aka crl.pem crash bug)

[Ken Bantoft]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hi Andreas,

We (myself + dhr + mcr) tracked down the 'crl.pem' bug to an off-by-one 
error in x509.c, which is still present in Strongswan 2.0.2

Attached is a patch, and we switched to snprintf too, as dhr's been 
snprintf happy for dealing with /proc stuff, as it's all been changed by 
2.4.25 kernels and higher.

This is against 1.4.8, but Strongswan 2.0.2 has the same code.

Note that in running pluto through valgrind, I also uncovered a read 
error buried in the id.c code:

==22686== Invalid read of size 1
==22686==    at 0x4207A995: strnlen (in /lib/tls/libc-2.3.2.so)
==22686==    by 0x420478C7: _IO_vfprintf_internal (in /lib/tls/libc-2.3.2.so)
==22686==    by 0x4206A363: _IO_vsnprintf (in /lib/tls/libc-2.3.2.so)
==22686==    by 0x4204F513: snprintf (in /lib/tls/libc-2.3.2.so)
==22686==    by 0x805D4D9: idtoa (id.c:325)
==22686==    by 0x805CD9C: calc_myid_str (id.c:82)
==22686==    by 0x805D008: set_myFQDN (id.c:142)
==22686==    by 0x805CD65: init_id (id.c:73)
==22686==  Address 0x75080362 is 0 bytes after a block of size 22 alloc'd
==22686==    at 0x7501D858: malloc (vg_replace_malloc.c:105)
==22686==    by 0x809676C: clone_bytes (alloc.c:179)
==22686==    by 0x805CFEC: set_myFQDN (id.c:140)


The actual line that valgrind catches is


    case ID_FQDN:
        n = snprintf(dst, dstlen, "@%.*s", (int)id->name.len, id->name.ptr);
        break;

However since variations of that snprintf() are above/below, it's really 
elsewhere.

Ken


Index: x509.c
===================================================================
RCS file: /xelerance/master/openswan-2/programs/pluto/x509.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -r1.9 -r1.10
- --- x509.c	21 Mar 2004 05:00:18 -0000	1.9
+++ x509.c	20 Apr 2004 00:27:29 -0000	1.10
@@ -1767,12 +1767,11 @@
 		if (load_coded_file(filename, NULL, "crl", &blob, &pgp))
 		{
 		    chunk_t crl_uri;
- -		    crl_uri.len = 7 + sizeof(CRL_PATH) + strlen(filename);
+		    crl_uri.len = 8 + sizeof(CRL_PATH) + strlen(filename);
 		    crl_uri.ptr = alloc_bytes(crl_uri.len + 1, "crl uri");
 
 		    /* build CRL file URI */
- -		    sprintf(crl_uri.ptr, "file://%s/%s", CRL_PATH, filename);
- -
+		    snprintf(crl_uri.ptr, crl_uri.len, "file://%s/%s", CRL_PATH, filename);
 		    insert_crl(blob, crl_uri);
 		}
 		free(filelist[n]);

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQFAhHc4PiOgilmwgkgRAu0tAKCJfVsW6p/Q3Lrt2apgeqIOh4mB6wCeOpRF
va8GQ9ABm0Z0CgYJiY2Tk5Q=
=LNxZ
-----END PGP SIGNATURE-----