libsec: cleanup x509 and tlshand

- add overflow checks for newbytes(), newbits(), newints()
- remove suspicious nil check from estrdup()
- remove useless nil checks before free

sys/src/libsec/port/tlshand.c

@@ -325,7 +325,6 @@ static Bytes* mptobytes(mpint* big);
 static mpint* bytestomp(Bytes* bytes);
 static void freebytes(Bytes* b);
 static Ints* newints(int len);
-static Ints* makeints(int* buf, int len);
 static void freeints(Ints* b);
 
 /* x509.c */
@@ -2228,10 +2227,9 @@ serverMasterSecret(TlsSec *sec, Bytes *epm)
 	// Hence the fprint(2,) can't be replaced by tlsError(), which sends an Alert msg to the client.
 	if(sec->ok < 0 || pm == nil || get16(pm->data) != sec->clientVers){
 		fprint(2, "serverMasterSecret failed ok=%d pm=%p pmvers=%x cvers=%x nepm=%d\n",
-			sec->ok, pm, pm ? get16(pm->data) : -1, sec->clientVers, epm->len);
+			sec->ok, pm, pm != nil ? get16(pm->data) : -1, sec->clientVers, epm->len);
 		sec->ok = -1;
-		if(pm != nil)
+		freebytes(pm);
-			freebytes(pm);
 		pm = newbytes(MasterSecretSize);
 		genrandom(pm->data, MasterSecretSize);
 	}
@@ -2537,6 +2535,8 @@ newbytes(int len)
 {
 	Bytes* ans;
 
+	if(len < 0)
+		abort();
 	ans = (Bytes*)emalloc(OFFSET(data[0], Bytes) + len);
 	ans->len = len;
 	return ans;
@@ -2551,16 +2551,14 @@ makebytes(uchar* buf, int len)
 	Bytes* ans;
 
 	ans = newbytes(len);
-	if(len > 0)
+	memmove(ans->data, buf, len);
-		memmove(ans->data, buf, len);
 	return ans;
 }
 
 static void
 freebytes(Bytes* b)
 {
-	if(b != nil)
+	free(b);
-		free(b);
 }
 
 /* len is number of ints */
@@ -2569,25 +2567,15 @@ newints(int len)
 {
 	Ints* ans;
 
+	if(len < 0 || len > ((uint)-1>>1)/sizeof(int))
+		abort();
 	ans = (Ints*)emalloc(OFFSET(data[0], Ints) + len*sizeof(int));
 	ans->len = len;
 	return ans;
 }
 
-static Ints*
-makeints(int* buf, int len)
-{
-	Ints* ans;
-
-	ans = newints(len);
-	if(len > 0)
-		memmove(ans->data, buf, len*sizeof(int));
-	return ans;
-}
-
 static void
 freeints(Ints* b)
 {
-	if(b != nil)
+	free(b);
-		free(b);
 }

sys/src/libsec/port/x509.c

@@ -177,14 +177,13 @@ emalloc(int n)
 static char*
 estrdup(char *s)
 {
-	char *d, *d0;
+	char *d;
+	int n;
 
-	if(!s)
+	n = strlen(s)+1;
-		return 0;
+	d = emalloc(n);
-	d = d0 = emalloc(strlen(s)+1);
+	memmove(d, s, n);
-	while(*d++ = *s++)
+	return d;
-		;
-	return d0;
 }
 
 
@@ -1287,6 +1286,8 @@ newbytes(int len)
 {
 	Bytes* ans;
 
+	if(len < 0)
+		abort();
 	ans = (Bytes*)emalloc(OFFSETOF(data[0], Bytes) + len);
 	ans->len = len;
 	return ans;
@@ -1308,8 +1309,7 @@ makebytes(uchar* buf, int len)
 static void
 freebytes(Bytes* b)
 {
-	if(b != nil)
+	free(b);
-		free(b);
 }
 
 /*
@@ -1347,6 +1347,8 @@ newints(int len)
 {
 	Ints* ans;
 
+	if(len < 0 || len > ((uint)-1>>1)/sizeof(int))
+		abort();
 	ans = (Ints*)emalloc(OFFSETOF(data[0], Ints) + len*sizeof(int));
 	ans->len = len;
 	return ans;
@@ -1358,16 +1360,14 @@ makeints(int* buf, int len)
 	Ints* ans;
 
 	ans = newints(len);
-	if(len > 0)
+	memmove(ans->data, buf, len*sizeof(int));
-		memmove(ans->data, buf, len*sizeof(int));
 	return ans;
 }
 
 static void
 freeints(Ints* b)
 {
-	if(b != nil)
+	free(b);
-		free(b);
 }
 
 /* len is number of bytes */
@@ -1376,6 +1376,8 @@ newbits(int len)
 {
 	Bits* ans;
 
+	if(len < 0)
+		abort();
 	ans = (Bits*)emalloc(OFFSETOF(data[0], Bits) + len);
 	ans->len = len;
 	ans->unusedbits = 0;
@@ -1396,8 +1398,7 @@ makebits(uchar* buf, int len, int unusedbits)
 static void
 freebits(Bits* b)
 {
-	if(b != nil)
+	free(b);
-		free(b);
 }
 
 static Elist*
@@ -1471,15 +1472,13 @@ freevalfields(Value* v)
 		el = v->u.seqval;
 		for(l = el; l != nil; l = l->tl)
 			freevalfields(&l->hd.val);
-		if(el)
+		freeelist(el);
-			freeelist(el);
 		break;
 	case VSet:
 		el = v->u.setval;
 		for(l = el; l != nil; l = l->tl)
 			freevalfields(&l->hd.val);
-		if(el)
+		freeelist(el);
-			freeelist(el);
 		break;
 	}
 }
@@ -1669,15 +1668,12 @@ static DigestAlg *digestalg[NUMALGS+1] = {
 static void
 freecert(CertX509* c)
 {
-	if(!c) return;
+	if(c == nil)
-	if(c->issuer != nil)
+		return;
-		free(c->issuer);
+	free(c->issuer);
-	if(c->validity_start != nil)
+	free(c->validity_start);
-		free(c->validity_start);
+	free(c->validity_end);
-	if(c->validity_end != nil)
+	free(c->subject);
-		free(c->validity_end);
-	if(c->subject != nil)
-		free(c->subject);
 	freebytes(c->publickey);
 	freebytes(c->signature);
 	free(c);
@@ -1925,12 +1921,10 @@ decode_rsapubkey(Bytes* a)
 	if(mp == nil)
 		goto errret;
 
-	if(l != nil)
+	freeelist(l);
-		freeelist(l);
 	return key;
 errret:
-	if(l != nil)
+	freeelist(l);
-		freeelist(l);
 	rsapubfree(key);
 	return nil;
 }
@@ -2222,10 +2216,8 @@ verify_signature(Bytes* signature, RSApub *pk, uchar *edigest, int edigestlen, E
 		err = "digests did not match";
 
 end:
-	if(pkcs1 != nil)
+	mpfree(pkcs1);
-		mpfree(pkcs1);
+	free(pkcs1buf);
-	if(pkcs1buf != nil)
-		free(pkcs1buf);
 	return err;
 }
 
@@ -2366,7 +2358,7 @@ mkutc(long t)
 	e.tag.class = Universal;
 	e.tag.num = UTCTime;
 	e.val.tag = VString;
-	snprint(utc, 50, "%.2d%.2d%.2d%.2d%.2d%.2dZ",
+	snprint(utc, sizeof(utc), "%.2d%.2d%.2d%.2d%.2d%.2dZ",
 		tm->year % 100, tm->mon+1, tm->mday, tm->hour, tm->min, tm->sec);
 	e.val.u.stringval = estrdup(utc);
 	return e;