From 6d3e97e83a7d61cbb2f5109efb4b519383a55712 Mon Sep 17 00:00:00 2001 From: Eugene Syromyatnikov Date: Tue, 28 Jun 2022 16:55:49 +0200 Subject: [PATCH] util: add offs sanity check to print_clock_t While it is not strictly needed right now, the code that uses the calculated offs value lacks any checks for possible buf overruns, which is not defensive enough, so let's add them. Reported by covscan: Error: OVERRUN (CWE-119): strace-5.18/src/util.c:248: assignment: Assigning: "offs" = "ilog10(val / clk_tck)". The value of "offs" is now between 16 and 31 (inclusive). strace-5.18/src/util.c:249: overrun-local: Overrunning array of 30 bytes at byte offset 31 by dereferencing pointer "buf + offs". [Note: The source code implementation of the function has been overridden by a builtin model.] Error: OVERRUN (CWE-119): strace-5.18/src/util.c:248: assignment: Assigning: "offs" = "ilog10(val / clk_tck)". The value of "offs" is now between 16 and 31 (inclusive). strace-5.18/src/util.c:253: overrun-buffer-arg: Overrunning array "buf" of 30 bytes by passing it to a function which accesses it at byte offset 32 using argument "offs + 2UL" (which evaluates to 33). [Note: The source code implementation of the function has been overridden by a builtin model.] Error: OVERRUN (CWE-119): strace-5.18/src/util.c:248: assignment: Assigning: "offs" = "ilog10(val / clk_tck)". The value of "offs" is now between 16 and 31 (inclusive). strace-5.18/src/util.c:254: overrun-local: Overrunning array "buf" of 30 bytes at byte offset 32 using index "offs + 1UL" (which evaluates to 32). * src/util.c (print_clock_t): Add check that offs is small enough for it and "offs + 2" not to overrun buf. --- src/util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util.c b/src/util.c index 5f87acb..93aa7b3 100644 --- a/src/util.c +++ b/src/util.c @@ -246,6 +246,14 @@ print_clock_t(uint64_t val) */ char buf[sizeof(uint64_t) * 3 + sizeof("0.0 s")]; size_t offs = ilog10(val / clk_tck); + /* + * This check is mostly to appease covscan, which thinks + * that offs can go as high as 31 (it cannot), but since + * there is no proper sanity checks against offs overrunning + * buf down the code, it may as well be here. + */ + if (offs > (sizeof(buf) - sizeof("0.0 s"))) + return; int ret = snprintf(buf + offs, sizeof(buf) - offs, "%.*f s", frac_width, (double) (val % clk_tck) / clk_tck); -- 2.1.4