citusdata / pg_cron

Run periodic jobs in PostgreSQL
PostgreSQL License
2.9k stars 195 forks source link

OpenBSD crond / crontab set_range() heap underflow (CVE-2024-43688) #351

Closed df7cb closed 3 months ago

df7cb commented 3 months ago

The vulnerable code from CVE-2024-43688 is present in pg_cron 1.6.x, so I think it is affected, even if I could not immediately reproduce the problem. Please give this a check.

https://www.supernetworks.org/CVE-2024-43688/openbsd-cron-heap-underflow.txt


Vulnerability Details

There is a potentially exploitable heap underflow in recent versions of Vixie Cron, that affects both the cron daemon and the crontab command. An attacker can use this vulnerability to obtain root on OpenBSD 7.4 and 7.5.

Crontab entries begin with five fields that specify minute, hour, day of month, month, and day of week. To allow for more advanced command scheduling, each of these fields can accept a range of numbers and/or step values. A concise example from the man page on both ranges and step values is "0-23/2 can be used in the hours field to specify command execution every other hour", with a range being 0-23 and a step value being 2.

In May of 2023 [1], significant changes were made to the range and step handling code of a crontab entry in Vixie Cron. A new function, set_range() was introduced in entry.c. This patch was incorporated into OpenBSD in June of 2023 [2]. Let's take a quick look at set_range:

1 static int
2 set_range(bitstr_t *bits, int low, int high, int start, int stop, int step) {
      [ ... ]
5   if (start < low || stop > high)
6       return (EOF);
7   start -= low;
8   stop -= low;

9       if (step == 1) {
10      bit_nset(bits, start, stop);
11  } else {
12      for (int i = start; i <= stop; i += step)
13          bit_set(bits, i);
14  }
15  return (OK);
16 }

The for loop on line 12 will add the step value provided in the crontab entry to i. This will then be used by bit_set as an index in the bits bitstring. As a result, an attacker can modify arbitrary values in memory, provided that they are within 256 megabytes of bits.

This condition only occurs with the step value as the range values are all sanity checked to ensure they don't exceed the maximum value of a field (e.g. the minutes value cannot exceed 59).

While OpenBSD implemented a privsep cron, where crontab is setgid cron instead of setuid root, this same vulnerability can be used to target both crontab and cron.

PoC — The following shell script will trigger the vulnerability.

#!/bin/sh
crontab - << EOF
0-0/2147494407 * * * * test
EOF

-

[1] https://github.com/vixie/cron/commit/62a064fd775cd682426176bab002a7d54a6b5bfc [2] https://github.com/openbsd/src/commit/314287cb0fc4d196d759a12ff1da4f7c4c004504

marcoslot commented 3 months ago

We were already rejecting negative/overflowed steps:

            /* get the number following the dash
             */
        ch = get_number(&num3, 0, PPC_NULL, ch, file);
        if (ch == EOF || num3 <= 0)
            return EOF;

https://github.com/citusdata/pg_cron/blob/main/src/entry.c#L396

Somehow Vixie Cron has == 0 there, so it failed to catch the issue.

        /* get the step size -- note: we don't pass the
         * names here, because the number is not an
         * element id, it's a step size.  'low' is
         * sent as a 0 since there is no offset either.
         */
        ch = get_number(&num3, 0, PPC_NULL, ch, file, ", \t\n");
        if (ch == EOF || num3 == 0)
            return (EOF);

https://github.com/vixie/cron/blob/9cc8ab1087bb9ab861dd5595c41200683c9f6712/entry.c#L543

So this does not reproduce on pg_cron, but will add some hardening anyway because large step sizes seem like an unnecessary liability.

marcoslot commented 3 months ago

A case that would trigger the issue if we did not have the right check already is:

select cron.schedule('*/2147483648 * * * *', 'SELECT 1');
ERROR:  invalid schedule: */2147483648 * * * *
HINT:  Use cron format (e.g. 5 4 * * *), or interval format '[1-59] seconds'

which throws an error because of the num3 <= 0 check (since value overflows)

df7cb commented 3 months ago

Thanks!