shithub: riscv

Info  •  Files  •  Log  •  Branches

libc: fix out of bounds access in dirpackage(), simplify

- dirpackage() was not checking if the stat entry lies within
the buffer. fixed.

- simplify dirpackage(), as we process all the bytes from
the buffer, we do not need to track "ss" here as its the same
as "ts".

- zero Dir* array pointer early in dirread() and dirreadall()
and avoid calling dirpackage on <= buffer length.

--- a/sys/src/libc/9sys/dirread.c

+++ b/sys/src/libc/9sys/dirread.c

@@ -7,29 +7,22 @@

 dirpackage(uchar *buf, long ts, Dir **d)

 {

 	char *s;

-	long ss, i, n, nn, m;

+	long i, n, nn, m;

-	*d = nil;

-	if(ts <= 0)

-		return 0;

-

 	/*

 	 * first find number of all stats, check they look like stats, & size all associated strings

 	 */

-	ss = 0;

 	n = 0;

 	for(i = 0; i < ts; i += m){

+		if(i+BIT16SZ >= ts)

+			return -1;

 		m = BIT16SZ + GBIT16(&buf[i]);

-		if(statcheck(&buf[i], m) < 0)

-			break;

-		ss += m;

+		if(i+m > ts || statcheck(&buf[i], m) < 0)

+			return -1;

 		n++;

 	}

-	if(i != ts)

-		return -1;

-

-	*d = malloc(n * sizeof(Dir) + ss);

+	*d = malloc(n * sizeof(Dir) + ts);

 	if(*d == nil)

 		return -1;

@@ -39,8 +32,8 @@

 	s = (char*)*d + n * sizeof(Dir);

 	nn = 0;

 	for(i = 0; i < ts; i += m){

-		m = BIT16SZ + GBIT16((uchar*)&buf[i]);

-		if(nn >= n || convM2D(&buf[i], m, *d + nn, s) != m){

+		m = BIT16SZ + GBIT16(&buf[i]);

+		if(i+m > ts || nn >= n || convM2D(&buf[i], m, *d + nn, s) != m){

 			free(*d);

 			*d = nil;

 			return -1;

@@ -58,11 +51,12 @@

 	uchar *buf;

 	long ts;

+	*d = nil;

 	buf = malloc(DIRMAX);

 	if(buf == nil)

 		return -1;

 	ts = read(fd, buf, DIRMAX);

-	if(ts >= 0)

+	if(ts > 0)

 		ts = dirpackage(buf, ts, d);

 	free(buf);

 	return ts;

@@ -74,6 +68,7 @@

 	uchar *buf, *nbuf;

 	long n, ts;

+	*d = nil;

 	buf = nil;

 	ts = 0;

 	for(;;){

@@ -88,7 +83,7 @@

 			break;

 		ts += n;

 	}

-	if(ts >= 0)

+	if(ts > 0)

 		ts = dirpackage(buf, ts, d);

 	free(buf);

 	if(ts == 0 && n < 0)