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