Hi,
ac(8) has 2 separate problems where it can segfault if we pass a bogus file to
read connect time data instead of the default /var/log/wtmp. I've looked at all
other BSDs and since the code is based on the same source it seems they suffer
from the same problem as well, although I didn't test them.
1) $ ac -w ./ac.c
Segmentation fault (core dumped)
This one will happen if the structure is big enough to write memory
out-of-bounds through strlcpy(3) inside update_user(). The easiest way to check
something is really wrong is to just check if secs is < 0 which should never
happen. Maybe also adding || secs > LLONG_MAX as well wouldn't hurt? If we also
use -p sometimes we can even see part of the contents of that file.
$ ac -p -w ./ac.c
opyright (c) 1994 Christopher G. -1360794071379245.75
etain the above copyright
* -1441250938215251.25
user_list *Users = NULL;
static -1653128621001371.00
-") == 0)
fp = stdin;
else if -1655483745007375.25
login time for 24hr period in de -1681363511581523.25
total 2456170264876095.00
With the diff this will be displayed instead:
$ obj/ac -w ./ac.c
ac: broken record. secs is -6052908641693484365
2) $ ac -w ./ac.c -d
Segmentation fault (core dumped)
This will make it segfault on a different part inside of ac() function because
of assigning a variable or checking its value with a null dereference. This
happens because localtime(3) was given a bogus input too large and therefore
returns NULL so we have to check that value before proceeding further.
After applying the diff this is what will be displayed:
$ obj/ac -w ./ac.c -d
ac: localtime: Value too large to be stored in data type
Comments? OK?
Index: ac.c
===================================================================
RCS file: /cvs/src/usr.sbin/ac/ac.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 ac.c
--- ac.c 26 Apr 2018 12:42:51 -0000 1.25
+++ ac.c 18 Aug 2018 14:41:04 -0000
@@ -171,6 +171,9 @@ update_user(struct user_list *head, char
{
struct user_list *up;
+ if (secs < 0)
+ errx(1, "broken record. secs is %lld", secs);
+
for (up = head; up != NULL; up = up->next) {
if (strncmp(up->name, name, sizeof (up->name) - 1) == 0) {
up->secs += secs;
@@ -410,6 +413,8 @@ ac(FILE *fp)
prev = usr.ut_time;
if (Flags & AC_D) {
ltm = localtime(&usr.ut_time);
+ if (ltm == NULL)
+ err(1, "localtime");
if (day >= 0 && day != ltm->tm_yday) {
day = ltm->tm_yday;
/*
@@ -461,6 +466,8 @@ ac(FILE *fp)
if (Flags & AC_D) {
ltm = localtime(&usr.ut_time);
+ if (ltm == NULL)
+ err(1, "localtime");
if (day >= 0 && day != ltm->tm_yday) {
/*
* print yesterday's total