Hello,
just tried to reproduce the issue I think the problem here is just
with a short time format ('04:00').
In that case 5 characters are copied by strncpy in parse_timestamp
to timestamp variable. Unfortunately these 5 characters do not contain
the termination, therefore the following strcat appends after the
next "random" null byte. Therefore writing beyond the end of timestamp.
Attached patch tries to prevent this by explicitly terminating.
The long time format ('04:00:00') should not suffer from this flaw.
Kind regards,
Bernhard
# apt install sysstat-dbgsym
# valgrind sar -s 04:00
==18110== Memcheck, a memory error detector
==18110== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==18110== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==18110== Command: sar -s 04:00
==18110==
==18110== Conditional jump or move depends on uninitialised value(s)
==18110== at 0x4F2E449: __strcat_chk (strcat_chk.c:37)
==18110== by 0x10D26D: strcat (string3.h:148)
==18110== by 0x10D26D: parse_timestamp (sa_common.c:342)
==18110== by 0x10A8FF: main (sar.c:1237)
==18110==
From e41c706bd7fbb05000e921ffd8b8b630747cffc6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bernhard=20=C3=83=C2=9Cbelacker?= <bernha...@mailbox.org>
Date: Tue, 23 May 2017 20:31:56 +0200
Subject: Avoid buffer overflow in parse_timestamp by explicit termination.
https://bugs.debian.org/863197
# valgrind sar -s 04:00
==18110== Memcheck, a memory error detector
==18110== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==18110== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==18110== Command: sar -s 04:00
==18110==
==18110== Conditional jump or move depends on uninitialised value(s)
==18110== at 0x4F2E449: __strcat_chk (strcat_chk.c:37)
==18110== by 0x10D26D: strcat (string3.h:148)
==18110== by 0x10D26D: parse_timestamp (sa_common.c:342)
==18110== by 0x10A8FF: main (sar.c:1237)
==18110==
---
sa_common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sa_common.c b/sa_common.c
index d3ab242..4c35bff 100644
--- a/sa_common.c
+++ b/sa_common.c
@@ -339,6 +339,7 @@ int parse_timestamp(char *argv[], int *opt, struct tstamp *tse,
case 5:
strncpy(timestamp, argv[(*opt)++], 5);
+ timestamp[5] = '\0';
strcat(timestamp,":00");
break;
--
2.11.0
