Project

General

Profile

Actions

Bug #13657

closed

atq should be better at math

Added by Jason King 7 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
cmd - userland programs
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

In usr/src/cmd/cron/atq.c, we have the following:

       time_t  jobdate;
        struct tm *unpackeddate;
        char date[18];                          /* reformatted execution date */

        /*
         * Convert the file name to a date.
         */
        jobdate = num(&filename);
        unpackeddate = localtime(&jobdate);

        /* years since 1900 + base century 1900 */
        unpackeddate->tm_year += 1900;
        /*
         * Format the execution date of a job.
         */
        sprintf(date, "%3s %2d, %4d %02d:%02d", mthnames[unpackeddate->tm_mon],
            unpackeddate->tm_mday, unpackeddate->tm_year,
            unpackeddate->tm_hour, unpackeddate->tm_min);

        /*
         * Print the date the job will be executed.
         */
        printf("%-21.18s", date);
}

An astute observer will note that 3 + 1 + 2 + 2 + 4 + 1 + 2 + 1 + 2 = 18, and thus the sprintf() call will write a NUL immediately after the end of date.

Creating any at job and then running atq will trigger the stack smashing protection, but since the user has no way to cause atq to write more than that additional NUL at the end of date, we don't believe this presents a security issue, just a bug.

The fix is fairly trivial -- change the size of date to 19 bytes, though we should also use snprintf() here just since that's almost always (it not always) preferable to sprintf().

Actions #1

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 1360
Actions #2

Updated by Jason King 7 months ago

  • Gerrit CR deleted (1360)

To test this, I created a simple at job:

root@pi:/ws/illumos-gate (atq)# at now + 5min
at> echo hi
at> <EOT>
commands will be executed using /usr/bin/bash
job 1616172659.a at Fri Mar 19 11:50:59 2021

Without the change, atq crashes:

root@pi:/ws/illumos-gate (atq)# atq
 Rank     Execution Date     Owner      Job            Queue   Job Name
Abort (core dumped)

Using the fixed atq, atq no longer crashes:

oot@pi:/ws/illumos-gate (atq)# proto/root_i386/usr/bin/atq
 Rank     Execution Date     Owner      Job            Queue   Job Name
  1st   Mar 19, 2021 11:50   root       1616172659.a     a     stdin
Actions #3

Updated by Jason King 7 months ago

  • Gerrit CR set to 1360
Actions #4

Updated by Jason King 7 months ago

Note that with or without the change, the jobs still complete successfully:

root@pi:/ws/illumos-gate (ixgbe)# mail
From root@pi Fri Mar 19 11:50:59 2021
To: root
Subject: At <root@pi> /var/spool/cron/atjobs/1616172659.a
Content-Length: 3
Date: Fri, 19 Mar 2021 11:50:59 -0500
Message-Id: <6054d673.30aec.2e9d5a04@pi>
From: <root@pi>

hi

The only issue is using atq to view any pending jobs will crash (at -l also appears to be unaffected by this).

Actions #5

Updated by Electric Monk 7 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 1d5eda342f972d3bf9b3bec5dc9d1103beb74cb7

commit  1d5eda342f972d3bf9b3bec5dc9d1103beb74cb7
Author: Jason King <jason.king@joyent.com>
Date:   2021-03-19T18:00:57.000Z

    13657 atq should be better at math
    Reviewed by: Dan McDonald <danmcd@joyent.com>
    Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
    Approved by: Robert Mustacchi <rm@fingolfin.org>

Actions

Also available in: Atom PDF