tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

buffer overflow in t_vis.c



I've found a one byte buffer overflow in t_vis.c.  It's caused by a
quite reasonable confusion about an undocumented behavior of always add
a '\0' terminating the dst string in strnunvisx().  This patch fixes the
test, but I think the behavior is confusing and should be documented in
addition to the requirement that the buffers by the same length.

-- Brooks

Author: Brooks Davis <brooks%one-eyed-alien.net@localhost>
Date:   Wed Apr 12 23:38:31 2017 +0000

    Fix a 1 byte buffer overflow in the strvis_basic test.
    
    dstbuf needs to be one byte longer than srcbuf to accommodate the NUL
    termination strunvisx always appends.  This behavior appears to be
    undocumented.
    
    Found with CHERI.

diff --git a/contrib/netbsd-tests/lib/libc/gen/t_vis.c b/contrib/netbsd-tests/lib/libc/gen/t_vis.c
index adb0930a300..9b9501d5b9c 100644
--- a/contrib/netbsd-tests/lib/libc/gen/t_vis.c
+++ b/contrib/netbsd-tests/lib/libc/gen/t_vis.c
@@ -68,7 +68,12 @@ ATF_TC_BODY(strvis_basic, tc)
 	char *srcbuf, *dstbuf, *visbuf;
 	unsigned int i, j;
 
-	ATF_REQUIRE((dstbuf = malloc(SIZE)) != NULL);
+	/*
+	 * NB: unvis(3) stats that dstbuf should be the size of visbuf
+	 * (the source buffer).  In practice, 1-byte larger than srcbuf
+	 * is sufficient to accommodate the undocumented '\0' termination.
+	 */
+	ATF_REQUIRE((dstbuf = malloc(SIZE + 1)) != NULL);
 	ATF_REQUIRE((srcbuf = malloc(SIZE)) != NULL);
 	ATF_REQUIRE((visbuf = malloc(SIZE * 4 + 1)) != NULL);
 

Attachment: signature.asc
Description: PGP signature



Home | Main Index | Thread Index | Old Index