Module Name: src Committed By: rillig Date: Thu Feb 15 23:48:51 UTC 2024
Modified Files: src/common/lib/libutil: snprintb.c src/tests/lib/libutil: t_snprintb.c Log Message: snprintb: fix string termination (since today) In the previous commit, I had accidentally only run the tests for snprintb_m but not those for snprintb, thereby missing a newly introduced bug that would not null-terminate the resulting strings. Add more tests to cover similar situations in which the buffer is too small to contain the complete output. To generate a diff of this commit: cvs rdiff -u -r1.24 -r1.25 src/common/lib/libutil/snprintb.c cvs rdiff -u -r1.14 -r1.15 src/tests/lib/libutil/t_snprintb.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/common/lib/libutil/snprintb.c diff -u src/common/lib/libutil/snprintb.c:1.24 src/common/lib/libutil/snprintb.c:1.25 --- src/common/lib/libutil/snprintb.c:1.24 Thu Feb 15 22:48:58 2024 +++ src/common/lib/libutil/snprintb.c Thu Feb 15 23:48:51 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: snprintb.c,v 1.24 2024/02/15 22:48:58 rillig Exp $ */ +/* $NetBSD: snprintb.c,v 1.25 2024/02/15 23:48:51 rillig Exp $ */ /*- * Copyright (c) 2002 The NetBSD Foundation, Inc. @@ -41,7 +41,7 @@ # include <sys/cdefs.h> # if defined(LIBC_SCCS) && !defined(lint) -__RCSID("$NetBSD: snprintb.c,v 1.24 2024/02/15 22:48:58 rillig Exp $"); +__RCSID("$NetBSD: snprintb.c,v 1.25 2024/02/15 23:48:51 rillig Exp $"); # endif # include <sys/types.h> @@ -51,7 +51,7 @@ __RCSID("$NetBSD: snprintb.c,v 1.24 2024 # include <errno.h> # else /* ! _KERNEL */ # include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: snprintb.c,v 1.24 2024/02/15 22:48:58 rillig Exp $"); +__KERNEL_RCSID(0, "$NetBSD: snprintb.c,v 1.25 2024/02/15 23:48:51 rillig Exp $"); # include <sys/param.h> # include <sys/inttypes.h> # include <sys/systm.h> @@ -275,13 +275,16 @@ snprintb_m(char *buf, size_t bufsize, co if (sep != '<') STORE('>'); terminate: - STORE('\0'); - if (l_max != 0) { + if (l_max > 0) { + bufsize++; STORE('\0'); - t_len--; - buf[bufsize] = '\0'; + if ((size_t)t_len >= bufsize && bufsize > 1) + buf[bufsize - 2] = '\0'; } - return t_len; + STORE('\0'); + if ((size_t)t_len >= bufsize && bufsize > 0) + buf[bufsize - 1] = '\0'; + return t_len - 1; internal: #ifndef _KERNEL errno = EINVAL; Index: src/tests/lib/libutil/t_snprintb.c diff -u src/tests/lib/libutil/t_snprintb.c:1.14 src/tests/lib/libutil/t_snprintb.c:1.15 --- src/tests/lib/libutil/t_snprintb.c:1.14 Thu Feb 15 22:37:10 2024 +++ src/tests/lib/libutil/t_snprintb.c Thu Feb 15 23:48:51 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: t_snprintb.c,v 1.14 2024/02/15 22:37:10 rillig Exp $ */ +/* $NetBSD: t_snprintb.c,v 1.15 2024/02/15 23:48:51 rillig Exp $ */ /* * Copyright (c) 2002, 2004, 2008, 2010, 2024 The NetBSD Foundation, Inc. @@ -32,7 +32,7 @@ #include <sys/cdefs.h> __COPYRIGHT("@(#) Copyright (c) 2008, 2010\ The NetBSD Foundation, inc. All rights reserved."); -__RCSID("$NetBSD: t_snprintb.c,v 1.14 2024/02/15 22:37:10 rillig Exp $"); +__RCSID("$NetBSD: t_snprintb.c,v 1.15 2024/02/15 23:48:51 rillig Exp $"); #include <stdio.h> #include <string.h> @@ -42,19 +42,37 @@ __RCSID("$NetBSD: t_snprintb.c,v 1.14 20 #include <atf-c.h> static const char * -vis_arr(const char *arr, size_t arrlen) +vis_arr(const char *arr, size_t arrsize) { static char buf[6][1024]; static size_t i; i = (i + 1) % (sizeof(buf) / sizeof(buf[0])); - int rv = strnvisx(buf[i], sizeof(buf[i]), arr, arrlen, + int rv = strnvisx(buf[i], sizeof(buf[i]), arr, arrsize, VIS_WHITE | VIS_OCTAL); - ATF_REQUIRE_MSG(rv >= 0, "strnvisx failed for length %zu", arrlen); + ATF_REQUIRE_MSG(rv >= 0, "strnvisx failed for size %zu", arrsize); return buf[i]; } static void +check_unmodified_loc(const char *file, size_t line, + const char *arr, size_t begin, size_t end) +{ + size_t mod_begin = begin, mod_end = end; + while (mod_begin < mod_end && arr[mod_begin] == 'Z') + mod_begin++; + while (mod_begin < mod_end && arr[mod_end - 1] == 'Z') + mod_end--; + ATF_CHECK_MSG( + mod_begin == mod_end, + "failed:\n" + "\ttest case: %s:%zu\n" + "\tout-of-bounds write from %zu to %zu: %s\n", + file, line, + mod_begin, mod_end, vis_arr(arr + mod_begin, mod_end - mod_begin)); +} + +static void h_snprintb_loc(const char *file, size_t line, size_t bufsize, const char *fmt, size_t fmtlen, uint64_t val, int exp_rv, const char *res, size_t reslen) @@ -65,6 +83,7 @@ h_snprintb_loc(const char *file, size_t // behavior due to out-of-range 'bp'. ATF_REQUIRE(bufsize > 0); ATF_REQUIRE(bufsize <= sizeof(buf)); + ATF_REQUIRE(reslen <= sizeof(buf)); memset(buf, 'Z', sizeof(buf)); int rv = snprintb(buf, bufsize, fmt, val); @@ -85,6 +104,7 @@ h_snprintb_loc(const char *file, size_t (uintmax_t)val, exp_rv, vis_arr(res, reslen), rv, vis_arr(buf, reslen)); + check_unmodified_loc(file, line, buf, reslen + 1, sizeof(buf)); } #define h_snprintb_len(bufsize, fmt, val, exp_rv, res) \ @@ -569,6 +589,52 @@ ATF_TC_BODY(snprintb, tc) ch, expected); } + + // new-style format, small buffer +#if 0 + // FIXME: Calling snprintb with buffer size 0 invokes undefined + // behavior due to out-of-bounds 'bp' pointer. + h_snprintb_len( + 0, "\177\020", 0, + 1, "ZZZ"); +#endif + h_snprintb_len( + 1, "\177\020", 0, + 1, "\0ZZZ"); + h_snprintb_len( + 2, "\177\020", 0, + 1, "0\0ZZZ"); + h_snprintb_len( + 3, "\177\020", 0, + 1, "0\0ZZZ"); + h_snprintb_len( + 3, "\177\020", 7, + 3, "0x\0ZZZ"); + h_snprintb_len( + 4, "\177\020", 7, + 3, "0x7\0ZZZ"); + h_snprintb_len( + 7, "\177\020b\000lsb\0", 7, + 8, "0x7<ls\0ZZZ"); + h_snprintb_len( + 8, "\177\020b\000lsb\0", 7, + 8, "0x7<lsb\0ZZZ"); + h_snprintb_len( + 9, "\177\020b\000lsb\0", 7, + 8, "0x7<lsb>\0ZZZ"); + h_snprintb_len( + 9, "\177\020b\000one\0b\001two\0", 7, + 12, "0x7<one,\0ZZZ"); + h_snprintb_len( + 10, "\177\020b\000one\0b\001two\0", 7, + 12, "0x7<one,t\0ZZZ"); + h_snprintb_len( + 12, "\177\020b\000one\0b\001two\0", 7, + 12, "0x7<one,two\0ZZZ"); + h_snprintb_len( + 13, "\177\020b\000one\0b\001two\0", 7, + 12, "0x7<one,two>\0ZZZ"); + } static void @@ -580,6 +646,7 @@ h_snprintb_m_loc(const char *file, size_ ATF_REQUIRE(bufsize > 1); ATF_REQUIRE(bufsize <= sizeof(buf)); + ATF_REQUIRE(reslen <= sizeof(buf)); memset(buf, 'Z', sizeof(buf)); int rv = snprintb_m(buf, bufsize, fmt, val, max); @@ -603,6 +670,7 @@ h_snprintb_m_loc(const char *file, size_ max, exp_rv, vis_arr(res, reslen), total, vis_arr(buf, reslen)); + check_unmodified_loc(file, line, buf, reslen + 1, sizeof(buf)); } #define h_snprintb_m_len(bufsize, fmt, val, line_max, exp_rv, res) \ @@ -675,6 +743,17 @@ ATF_TC_BODY(snprintb_m, tc) /* 140 */ "ZZZZZZZZZZ" ); + // new-style format, buffer too small for description + h_snprintb_m_len( + 8, + "\177\020" + "b\000bit1\0", + 0x0001, + 64, + 10, + "0x1<bi\0\0ZZZ" + ); + h_snprintb_m( "\177\020" "b\0LSB\0"