Many security improvements (CHATS) and a warning fix.

CHANGES

@@ -4,6 +4,91 @@
 
  Changes between 0.9.6h and 0.9.7  [XX xxx 2002]
 
+  *) Introduce safe string copy and catenation functions
+     (BUF_strlcpy() and BUF_strlcat()).
+     [Ben Laurie (CHATS) and Richard Levitte]
+
+  *) Avoid using fixed-size buffers for one-line DNs.
+     [Ben Laurie (CHATS)]
+
+  *) Add BUF_MEM_grow_clean() to avoid information leakage when
+     resizing buffers containing secrets, and use where appropriate.
+     [Ben Laurie (CHATS)]
+
+  *) Avoid using fixed size buffers for configuration file location.
+     [Ben Laurie (CHATS)]
+
+  *) Avoid filename truncation for various CA files.
+     [Ben Laurie (CHATS)]
+
+  *) Use sizeof in preference to magic numbers.
+     [Ben Laurie (CHATS)]
+
+  *) Avoid filename truncation in cert requests.
+     [Ben Laurie (CHATS)]
+
+  *) New OPENSSL_assert() macro (similar to assert(), but enabled
+     unconditionally).
+     [Ben Laurie (CHATS)]
+
+  *) Add assertions to check for (supposedly impossible) buffer
+     overflows.
+     [Ben Laurie (CHATS)]
+
+  *) Don't cache truncated DNS entries in the local cache (this could
+     potentially lead to a spoofing attack).
+     [Ben Laurie (CHATS)]
+
+  *) Fix various buffers to be large enough for hex/decimal
+     representations in a platform independent manner.
+     [Ben Laurie (CHATS)]
+
+  *) Add CRYPTO_realloc_clean() to avoid information leakage when
+     resizing buffers containing secrets, and use where appropriate.
+     [Ben Laurie (CHATS)]
+
+  *) Add BIO_indent() to avoid much slightly worrying code to do
+     indents.
+     [Ben Laurie (CHATS)]
+
+  *) Convert sprintf()/BIO_puts() to BIO_printf().
+     [Ben Laurie (CHATS)]
+
+  *) buffer_gets() could terminate with the buffer only half
+     full. Fixed.
+     [Ben Laurie (CHATS)]
+
+  *) Add assertions to prevent crypto functions from overflowing
+     internal buffers by having large block sizes, etc.
+     [Ben Laurie (CHATS)]
+
+  *) Eliminate unused copy of key in RC4.
+     [Ben Laurie (CHATS)]
+
+  *) Eliminate unused and incorrectly sized buffers for IV in pem.h.
+     [Ben Laurie (CHATS)]
+
+  *) Fix off-by-one error in EGD path.
+     [Ben Laurie (CHATS)]
+
+  *) If RANDFILE path is too long, ignore instead of truncating.
+     [Ben Laurie (CHATS)]
+
+  *) Eliminate unused and incorrectly sized X.509 structure
+     CBCParameter.
+     [Ben Laurie (CHATS)]
+
+  *) Eliminate unused and dangerous function knumber().
+     [Ben Laurie (CHATS)]
+
+  *) Eliminate unused and dangerous structure, KSSL_ERR.
+     [Ben Laurie (CHATS)]
+
+  *) Protect against overlong session ID context length in an encoded
+     session object. Since these are local, this does not appear to be
+     exploitable.
+     [Ben Laurie (CHATS)]
+
   *) Change from security patch (see 0.9.6e below) that did not affect
      the 0.9.6 release series:
 

apps/Makefile.ssl

@@ -762,30 +762,30 @@ rand.o: ../include/openssl/sha.h ../include/openssl/stack.h
 rand.o: ../include/openssl/symhacks.h ../include/openssl/txt_db.h
 rand.o: ../include/openssl/ui.h ../include/openssl/ui_compat.h
 rand.o: ../include/openssl/x509.h ../include/openssl/x509_vfy.h apps.h rand.c
-req.o: ../e_os.h ../include/openssl/aes.h ../include/openssl/asn1.h
-req.o: ../include/openssl/bio.h ../include/openssl/blowfish.h
-req.o: ../include/openssl/bn.h ../include/openssl/buffer.h
-req.o: ../include/openssl/cast.h ../include/openssl/conf.h
-req.o: ../include/openssl/crypto.h ../include/openssl/des.h
-req.o: ../include/openssl/des_old.h ../include/openssl/dh.h
-req.o: ../include/openssl/dsa.h ../include/openssl/e_os2.h
-req.o: ../include/openssl/engine.h ../include/openssl/err.h
-req.o: ../include/openssl/evp.h ../include/openssl/idea.h
-req.o: ../include/openssl/lhash.h ../include/openssl/md2.h
-req.o: ../include/openssl/md4.h ../include/openssl/md5.h
-req.o: ../include/openssl/mdc2.h ../include/openssl/obj_mac.h
-req.o: ../include/openssl/objects.h ../include/openssl/opensslconf.h
-req.o: ../include/openssl/opensslv.h ../include/openssl/ossl_typ.h
-req.o: ../include/openssl/pem.h ../include/openssl/pem2.h
-req.o: ../include/openssl/pkcs7.h ../include/openssl/rand.h
-req.o: ../include/openssl/rc2.h ../include/openssl/rc4.h
-req.o: ../include/openssl/rc5.h ../include/openssl/ripemd.h
-req.o: ../include/openssl/rsa.h ../include/openssl/safestack.h
-req.o: ../include/openssl/sha.h ../include/openssl/stack.h
-req.o: ../include/openssl/symhacks.h ../include/openssl/txt_db.h
-req.o: ../include/openssl/ui.h ../include/openssl/ui_compat.h
-req.o: ../include/openssl/x509.h ../include/openssl/x509_vfy.h
-req.o: ../include/openssl/x509v3.h apps.h req.c
+req.o: ../crypto/cryptlib.h ../e_os.h ../include/openssl/aes.h
+req.o: ../include/openssl/asn1.h ../include/openssl/bio.h
+req.o: ../include/openssl/blowfish.h ../include/openssl/bn.h
+req.o: ../include/openssl/buffer.h ../include/openssl/cast.h
+req.o: ../include/openssl/conf.h ../include/openssl/crypto.h
+req.o: ../include/openssl/des.h ../include/openssl/des_old.h
+req.o: ../include/openssl/dh.h ../include/openssl/dsa.h
+req.o: ../include/openssl/e_os2.h ../include/openssl/engine.h
+req.o: ../include/openssl/err.h ../include/openssl/evp.h
+req.o: ../include/openssl/idea.h ../include/openssl/lhash.h
+req.o: ../include/openssl/md2.h ../include/openssl/md4.h
+req.o: ../include/openssl/md5.h ../include/openssl/mdc2.h
+req.o: ../include/openssl/obj_mac.h ../include/openssl/objects.h
+req.o: ../include/openssl/opensslconf.h ../include/openssl/opensslv.h
+req.o: ../include/openssl/ossl_typ.h ../include/openssl/pem.h
+req.o: ../include/openssl/pem2.h ../include/openssl/pkcs7.h
+req.o: ../include/openssl/rand.h ../include/openssl/rc2.h
+req.o: ../include/openssl/rc4.h ../include/openssl/rc5.h
+req.o: ../include/openssl/ripemd.h ../include/openssl/rsa.h
+req.o: ../include/openssl/safestack.h ../include/openssl/sha.h
+req.o: ../include/openssl/stack.h ../include/openssl/symhacks.h
+req.o: ../include/openssl/txt_db.h ../include/openssl/ui.h
+req.o: ../include/openssl/ui_compat.h ../include/openssl/x509.h
+req.o: ../include/openssl/x509_vfy.h ../include/openssl/x509v3.h apps.h req.c
 rsa.o: ../e_os.h ../include/openssl/aes.h ../include/openssl/asn1.h
 rsa.o: ../include/openssl/bio.h ../include/openssl/blowfish.h
 rsa.o: ../include/openssl/bn.h ../include/openssl/buffer.h

apps/apps.c

@@ -337,8 +337,7 @@ void program_name(char *in, char *out, int size)
 		p++;
 	else
 		p=in;
-	strncpy(out,p,size-1);
-	out[size-1]='\0';
+	BUF_strlcpy(out,p,size);
 	}
 #endif
 #endif
@@ -447,16 +446,20 @@ int app_init(long mesgwin)
 
 int dump_cert_text (BIO *out, X509 *x)
 {
-	char buf[256];
-	X509_NAME_oneline(X509_get_subject_name(x),buf,256);
+	char *p;
+
+	p=X509_NAME_oneline(X509_get_subject_name(x),NULL,0);
 	BIO_puts(out,"subject=");
-	BIO_puts(out,buf);
+	BIO_puts(out,p);
+	OPENSSL_free(p);
 
-	X509_NAME_oneline(X509_get_issuer_name(x),buf,256);
-	BIO_puts(out,"\nissuer= ");
-	BIO_puts(out,buf);
+	p=X509_NAME_oneline(X509_get_issuer_name(x),NULL,0);
+	BIO_puts(out,"\nissuer=");
+	BIO_puts(out,p);
 	BIO_puts(out,"\n");
-        return 0;
+	OPENSSL_free(p);
+
+	return 0;
 }
 
 static int ui_open(UI *ui)
@@ -978,7 +981,7 @@ load_netscape_key(BIO *err, BIO *key, const char *file,
 		goto error;
 	for (;;)
 		{
-		if (!BUF_MEM_grow(buf,size+1024*10))
+		if (!BUF_MEM_grow_clean(buf,size+1024*10))
 			goto error;
 		i = BIO_read(key, &(buf->data[size]), 1024*10);
 		size += i;
@@ -1253,6 +1256,7 @@ void print_name(BIO *out, char *title, X509_NAME *nm, unsigned long lflags)
 	char *buf;
 	char mline = 0;
 	int indent = 0;
+
 	if(title) BIO_puts(out, title);
 	if((lflags & XN_FLAG_SEP_MASK) == XN_FLAG_SEP_MULTILINE) {
 		mline = 1;
@@ -1374,3 +1378,18 @@ int load_config(BIO *err, CONF *cnf)
 		}
 	return 1;
 	}
+
+char *make_config_name()
+	{
+	const char *t=X509_get_default_cert_area();
+	char *p;
+
+	p=OPENSSL_malloc(strlen(t)+strlen(OPENSSL_CONF)+2);
+	strcpy(p,t);
+#ifndef OPENSSL_SYS_VMS
+	strcat(p,"/");
+#endif
+	strcat(p,OPENSSL_CONF);
+
+	return p;
+	}

apps/apps.h

@@ -251,6 +251,7 @@ X509_STORE *setup_verify(BIO *bp, char *CAfile, char *CApath);
 ENGINE *setup_engine(BIO *err, const char *engine, int debug);
 
 int load_config(BIO *err, CONF *cnf);
+char *make_config_name(void);
 
 /* Functions defined in ca.c and also used in ocsp.c */
 int unpack_revinfo(ASN1_TIME **prevtm, int *preason, ASN1_OBJECT **phold,

apps/ca.c

@@ -334,6 +334,7 @@ int MAIN(int argc, char **argv)
 	MS_STATIC char buf[3][BSIZE];
 	char *randfile=NULL;
 	char *engine = NULL;
+	char *tofree=NULL;
 
 #ifdef EFENCE
 EF_PROTECT_FREE=1;
@@ -561,25 +562,26 @@ EF_ALIGNMENT=0;
 
 	ERR_load_crypto_strings();
 
-        e = setup_engine(bio_err, engine, 0);
+	e = setup_engine(bio_err, engine, 0);
 
 	/*****************************************************************/
+	tofree=NULL;
 	if (configfile == NULL) configfile = getenv("OPENSSL_CONF");
 	if (configfile == NULL) configfile = getenv("SSLEAY_CONF");
 	if (configfile == NULL)
 		{
-		/* We will just use 'buf[0]' as a temporary buffer.  */
+		const char *s=X509_get_default_cert_area();
+
 #ifdef OPENSSL_SYS_VMS
-		strncpy(buf[0],X509_get_default_cert_area(),
-			sizeof(buf[0])-1-sizeof(CONFIG_FILE));
+		tofree=OPENSSL_malloc(strlen(s)+sizeof(CONFIG_FILE));
+		strcpy(tofree,s);
 #else
-		strncpy(buf[0],X509_get_default_cert_area(),
-			sizeof(buf[0])-2-sizeof(CONFIG_FILE));
-		buf[0][sizeof(buf[0])-2-sizeof(CONFIG_FILE)]='\0';
-		strcat(buf[0],"/");
+		tofree=OPENSSL_malloc(strlen(s)+sizeof(CONFIG_FILE)+1);
+		strcpy(tofree,s);
+		strcat(tofree,"/");
 #endif
-		strcat(buf[0],CONFIG_FILE);
-		configfile=buf[0];
+		strcat(tofree,CONFIG_FILE);
+		configfile=tofree;
 		}
 
 	BIO_printf(bio_err,"Using configuration from %s\n",configfile);
@@ -594,6 +596,8 @@ EF_ALIGNMENT=0;
 				,errorline,configfile);
 		goto err;
 		}
+	if(tofree)
+		OPENSSL_free(tofree);
 
 	if (!load_config(bio_err, conf))
 		goto err;
@@ -1286,8 +1290,13 @@ EF_ALIGNMENT=0;
 
 			BIO_printf(bio_err,"Write out database with %d new entries\n",sk_X509_num(cert_sk));
 
-			strncpy(buf[0],serialfile,BSIZE-4);
-			buf[0][BSIZE-4]='\0';
+			if(strlen(serialfile) > BSIZE-5 || strlen(dbfile) > BSIZE-5)
+				{
+				BIO_printf(bio_err,"file name too long\n");
+				goto err;
+				}
+
+			strcpy(buf[0],serialfile);
 
 #ifdef OPENSSL_SYS_VMS
 			strcat(buf[0],"-new");
@@ -1297,8 +1306,7 @@ EF_ALIGNMENT=0;
 
 			if (!save_serial(buf[0],serial)) goto err;
 
-			strncpy(buf[1],dbfile,BSIZE-4);
-			buf[1][BSIZE-4]='\0';
+			strcpy(buf[1],dbfile);
 
 #ifdef OPENSSL_SYS_VMS
 			strcat(buf[1],"-new");
@@ -1328,8 +1336,13 @@ EF_ALIGNMENT=0;
 			j=x->cert_info->serialNumber->length;
 			p=(char *)x->cert_info->serialNumber->data;
 
-			strncpy(buf[2],outdir,BSIZE-(j*2)-6);
-			buf[2][BSIZE-(j*2)-6]='\0';
+			if(strlen(outdir) >= (j ? BSIZE-j*2-6 : BSIZE-8))
+				{
+				BIO_printf(bio_err,"certificate file name too long\n");
+				goto err;
+				}
+
+			strcpy(buf[2],outdir);
 
 #ifndef OPENSSL_SYS_VMS
 			strcat(buf[2],"/");
@@ -1563,8 +1576,13 @@ EF_ALIGNMENT=0;
 			if (j <= 0) goto err;
 			X509_free(revcert);
 
-			strncpy(buf[0],dbfile,BSIZE-4);
-			buf[0][BSIZE-4]='\0';
+			if(strlen(dbfile) > BSIZE-5)
+				{
+				BIO_printf(bio_err,"filename too long\n");
+				goto err;
+				}
+
+			strcpy(buf[0],dbfile);
 #ifndef OPENSSL_SYS_VMS
 			strcat(buf[0],".new");
 #else
@@ -1608,6 +1626,8 @@ EF_ALIGNMENT=0;
 	/*****************************************************************/
 	ret=0;
 err:
+	if(tofree)
+		OPENSSL_free(tofree);
 	BIO_free_all(Cout);
 	BIO_free_all(Sout);
 	BIO_free_all(out);

apps/ciphers.c

@@ -187,7 +187,7 @@ int MAIN(int argc, char **argv)
 			{
 			BIO_puts(STDout,SSL_CIPHER_description(
 				sk_SSL_CIPHER_value(sk,i),
-				buf,512));
+				buf,sizeof buf));
 			}
 		}
 

apps/dgst.c

@@ -117,7 +117,7 @@ int MAIN(int argc, char **argv)
 		goto end;
 
 	/* first check the program name */
-	program_name(argv[0],pname,PROG_NAME_SIZE);
+	program_name(argv[0],pname,sizeof pname);
 
 	md=EVP_get_digestbyname(pname);