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