shithub: riscv

Download patch

ref: 3b1a0ab1f34c8cb8c4e619d9b008207c22c10315
parent: c8ac5d7eb7999207c4fda314642df04325aafa68
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sat Apr 26 14:04:04 EDT 2014

tlshand: fix memory leaks, fix alloc element size for certs pointer array, error handling

--- a/sys/src/libsec/port/tlshand.c
+++ b/sys/src/libsec/port/tlshand.c
@@ -526,6 +526,10 @@
 		goto Err;
 	}
 	c->sec->rsapub = X509toRSApub(cert, ncert, nil, 0);
+	if(c->sec->rsapub == nil){
+		tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
+		goto Err;
+	}
 	msgClear(&m);
 
 	m.tag = HServerHello;
@@ -542,7 +546,7 @@
 	m.tag = HCertificate;
 	numcerts = countchain(chp);
 	m.u.certificate.ncert = 1 + numcerts;
-	m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes));
+	m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes*));
 	m.u.certificate.certs[0] = makebytes(cert, ncert);
 	for (i = 0; i < numcerts && chp; i++, chp = chp->next)
 		m.u.certificate.certs[i+1] = makebytes(chp->pem, chp->pemlen);
@@ -746,7 +750,7 @@
 	if(creq) {
 		if(cert != nil && certlen > 0){
 			m.u.certificate.ncert = 1;
-			m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes));
+			m.u.certificate.certs = emalloc(m.u.certificate.ncert * sizeof(Bytes*));
 			m.u.certificate.certs[0] = makebytes(cert, certlen);
 		}		
 		m.tag = HCertificate;
@@ -795,21 +799,22 @@
 			goto Err;
 		}
 		c->sec->rsapub = X509toRSApub(cert, certlen, nil, 0);
-		
+		if(c->sec->rsapub == nil){
+			tlsError(c, EHandshakeFailure, "invalid X509/rsa certificate");
+			goto Err;
+		}
+
 		paddedHashes = pkcs1padbuf(hshashes, 36, c->sec->rsapub->n);
 		signedMP = factotum_rsa_decrypt(c->sec->rpc, paddedHashes);
-		m.u.certificateVerify.signature = mptobytes(signedMP);
-		free(signedMP);
-
-		if(m.u.certificateVerify.signature == nil){
-			msgClear(&m);	
+		if(signedMP == nil){
+			tlsError(c, EHandshakeFailure, "factotum_rsa_decrypt: %r");
 			goto Err;
 		}
+		m.u.certificateVerify.signature = mptobytes(signedMP);
+		mpfree(signedMP);
 
-		if(!msgSend(c, &m, AFlush)){
-			msgClear(&m);
+		if(!msgSend(c, &m, AFlush))
 			goto Err;
-		}
 		msgClear(&m);
 	} 
 
@@ -1207,7 +1212,7 @@
 			if(nn > n)
 				goto Short;
 			m->u.certificate.ncert = i+1;
-			m->u.certificate.certs = erealloc(m->u.certificate.certs, (i+1)*sizeof(Bytes));
+			m->u.certificate.certs = erealloc(m->u.certificate.certs, (i+1)*sizeof(Bytes*));
 			m->u.certificate.certs[i] = makebytes(p, nn);
 			p += nn;
 			n -= nn;
@@ -1245,7 +1250,7 @@
 				goto Short;
 			m->u.certificateRequest.nca = i+1;
 			m->u.certificateRequest.cas = erealloc(
-				m->u.certificateRequest.cas, (i+1)*sizeof(Bytes));
+				m->u.certificateRequest.cas, (i+1)*sizeof(Bytes*));
 			m->u.certificateRequest.cas[i] = makebytes(p, nn);
 			p += nn;
 			n -= nn;
@@ -1729,13 +1734,14 @@
 	char *p;
 	int rv;
 
-	if((p = mptoa(cipher, 16, nil, 0)) == nil)
+	p = mptoa(cipher, 16, nil, 0);
+	mpfree(cipher);
+	if(p == nil)
 		return nil;
 	rv = auth_rpc(rpc, "write", p, strlen(p));
 	free(p);
 	if(rv != ARok || auth_rpc(rpc, "read", nil, 0) != ARok)
 		return nil;
-	mpfree(cipher);
 	return strtomp(rpc->arg, nil, 16, nil);
 }
 
@@ -2141,10 +2147,7 @@
 static mpint*
 bytestomp(Bytes* bytes)
 {
-	mpint* ans;
-
-	ans = betomp(bytes->data, bytes->len, nil);
-	return ans;
+	return betomp(bytes->data, bytes->len, nil);
 }
 
 /*
@@ -2153,16 +2156,13 @@
 static Bytes*
 mptobytes(mpint* big)
 {
-	int n, m;
-	uchar *a;
 	Bytes* ans;
+	int n;
 
-	a = nil;
 	n = (mpsignif(big)+7)/8;
-	m = mptobe(big, nil, n, &a);
-	ans = makebytes(a, m);
-	if(a != nil)
-		free(a);
+	if(n == 0) n = 1;
+	ans = newbytes(n);
+	ans->len = mptobe(big, ans->data, n, nil);
 	return ans;
 }
 
@@ -2180,6 +2180,7 @@
 	mpfree(x);
 	ybytes = mptobytes(y);
 	ylen = ybytes->len;
+	mpfree(y);
 
 	if(ylen < modlen) {
 		a = newbytes(modlen);
@@ -2195,7 +2196,6 @@
 		freebytes(ybytes);
 		ybytes = a;
 	}
-	mpfree(y);
 	return ybytes;
 }
 
@@ -2250,6 +2250,7 @@
 	if(y == nil)
 		return nil;
 	eb = mptobytes(y);
+	mpfree(y);
 	if(eb->len < modlen){ // pad on left with zeros
 		ans = newbytes(modlen);
 		memset(ans->data, 0, modlen-eb->len);
@@ -2278,9 +2279,8 @@
 	if(n==0)
 		n=1;
 	p = malloc(n);
-	if(p == nil){
-		exits("out of memory");
-	}
+	if(p == nil)
+		sysfatal("out of memory");
 	memset(p, 0, n);
 	setmalloctag(p, getcallerpc(&n));
 	return p;
@@ -2291,11 +2291,10 @@
 {
 	if(ReallocN == 0)
 		ReallocN = 1;
-	if(!ReallocP)
+	if(ReallocP == nil)
 		ReallocP = emalloc(ReallocN);
-	else if(!(ReallocP = realloc(ReallocP, ReallocN))){
-		exits("out of memory");
-	}
+	else if((ReallocP = realloc(ReallocP, ReallocN)) == nil)
+		sysfatal("out of memory");
 	setrealloctag(ReallocP, getcallerpc(&ReallocP));
 	return(ReallocP);
 }
@@ -2344,17 +2343,12 @@
 
 #define OFFSET(x, s) offsetof(s, x)
 
-/*
- * malloc and return a new Bytes structure capable of
- * holding len bytes. (len >= 0)
- * Used to use crypt_malloc, which aborts if malloc fails.
- */
 static Bytes*
 newbytes(int len)
 {
 	Bytes* ans;
 
-	ans = (Bytes*)malloc(OFFSET(data[0], Bytes) + len);
+	ans = (Bytes*)emalloc(OFFSET(data[0], Bytes) + len);
 	ans->len = len;
 	return ans;
 }
@@ -2368,7 +2362,8 @@
 	Bytes* ans;
 
 	ans = newbytes(len);
-	memmove(ans->data, buf, len);
+	if(len > 0)
+		memmove(ans->data, buf, len);
 	return ans;
 }
 
@@ -2385,7 +2380,7 @@
 {
 	Ints* ans;
 
-	ans = (Ints*)malloc(OFFSET(data[0], Ints) + len*sizeof(int));
+	ans = (Ints*)emalloc(OFFSET(data[0], Ints) + len*sizeof(int));
 	ans->len = len;
 	return ans;
 }
--- a/sys/src/libsec/port/x509.c
+++ b/sys/src/libsec/port/x509.c
@@ -169,9 +169,8 @@
 	if(n==0)
 		n=1;
 	p = malloc(n);
-	if(p == nil){
-		exits("out of memory");
-	}
+	if(p == nil)
+		sysfatal("out of memory");
 	memset(p, 0, n);
 	setmalloctag(p, getcallerpc(&n));
 	return p;