shithub: riscv

Info  •  Files  •  Log  •  Branches

libsec: cleanup x509 and tlshand

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

--- a/sys/src/libsec/port/tlshand.c

+++ b/sys/src/libsec/port/tlshand.c

@@ -325,7 +325,6 @@

 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 @@

 	// 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 @@

 {

 	Bytes* ans;

+	if(len < 0)

+		abort();

 	ans = (Bytes*)emalloc(OFFSET(data[0], Bytes) + len);

 	ans->len = len;

 	return ans;

@@ -2551,8 +2551,7 @@

 	Bytes* ans;

 	ans = newbytes(len);

-	if(len > 0)

-		memmove(ans->data, buf, len);

+	memmove(ans->data, buf, len);

 	return ans;

 }

@@ -2559,8 +2558,7 @@

 static void

 freebytes(Bytes* b)

 {

-	if(b != nil)

-		free(b);

+	free(b);

 }

 /* len is number of ints */

@@ -2569,25 +2567,15 @@

 {

 	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);

 }

--- a/sys/src/libsec/port/x509.c

+++ b/sys/src/libsec/port/x509.c

@@ -177,14 +177,13 @@

 static char*

 estrdup(char *s)

 {

-	char *d, *d0;

+	char *d;

+	int n;

-	if(!s)

-		return 0;

-	d = d0 = emalloc(strlen(s)+1);

-	while(*d++ = *s++)

-		;

-	return d0;

+	n = strlen(s)+1;

+	d = emalloc(n);

+	memmove(d, s, n);

+	return d;

 }

@@ -1287,6 +1286,8 @@

 {

 	Bytes* ans;

+	if(len < 0)

+		abort();

 	ans = (Bytes*)emalloc(OFFSETOF(data[0], Bytes) + len);

 	ans->len = len;

 	return ans;

@@ -1308,8 +1309,7 @@

 static void

 freebytes(Bytes* b)

 {

-	if(b != nil)

-		free(b);

+	free(b);

 }

 /*

@@ -1347,6 +1347,8 @@

 {

 	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,8 +1360,7 @@

 	Ints* ans;

 	ans = newints(len);

-	if(len > 0)

-		memmove(ans->data, buf, len*sizeof(int));

+	memmove(ans->data, buf, len*sizeof(int));

 	return ans;

 }

@@ -1366,8 +1367,7 @@

 static void

 freeints(Ints* b)

 {

-	if(b != nil)

-		free(b);

+	free(b);

 }

 /* len is number of bytes */

@@ -1376,6 +1376,8 @@

 {

 	Bits* ans;

+	if(len < 0)

+		abort();

 	ans = (Bits*)emalloc(OFFSETOF(data[0], Bits) + len);

 	ans->len = len;

 	ans->unusedbits = 0;

@@ -1396,8 +1398,7 @@

 static void

 freebits(Bits* b)

 {

-	if(b != nil)

-		free(b);

+	free(b);

 }

 static Elist*

@@ -1471,15 +1472,13 @@

 		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 void

 freecert(CertX509* c)

 {

-	if(!c) return;

-	if(c->issuer != nil)

-		free(c->issuer);

-	if(c->validity_start != nil)

-		free(c->validity_start);

-	if(c->validity_end != nil)

-		free(c->validity_end);

-	if(c->subject != nil)

-		free(c->subject);

+	if(c == nil)

+		return;

+	free(c->issuer);

+	free(c->validity_start);

+	free(c->validity_end);

+	free(c->subject);

 	freebytes(c->publickey);

 	freebytes(c->signature);

 	free(c);

@@ -1925,12 +1921,10 @@

 	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 @@

 		err = "digests did not match";

 end:

-	if(pkcs1 != nil)

-		mpfree(pkcs1);

-	if(pkcs1buf != nil)

-		free(pkcs1buf);

+	mpfree(pkcs1);

+	free(pkcs1buf);

 	return err;

 }

@@ -2366,7 +2358,7 @@

 	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;