Upstream fixes for @yearly, @monthly, @weekly, @daily and @hourly.

The bit_nset() macro was being called with the high value one too
large for the special strings.  There is no security impact due to
the layout of the bit strings but this was somewhat lucky.  This
introduces a set_range() function that performs range checks before
calling bit_nset().

1 files changed, 73 insertions, 35 deletions

diff --git a/usr.sbin/cron/entry.c b/usr.sbin/cron/entry.c
index e9b5028151d..2ac23e7f0db 100644
--- a/usr.sbin/cron/entry.c
+++ b/usr.sbin/cron/entry.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: entry.c,v 1.57 2023/06/04 17:27:27 millert Exp $	*/
+/*	$OpenBSD: entry.c,v 1.58 2023/06/13 15:36:21 millert Exp $	*/
 
 /*
  * Copyright 1988,1990,1993,1994 by Paul Vixie
@@ -69,7 +69,8 @@ static const char *DowNames[] = {
 static int	get_list(bitstr_t *, int, int, const char *[], int, FILE *),
 		get_range(bitstr_t *, int, int, const char *[], int, FILE *),
 		get_number(int *, int, const char *[], int, FILE *, const char *),
-		set_element(bitstr_t *, int, int, int);
+		set_element(bitstr_t *, int, int, int),
+		set_range(bitstr_t *, int, int, int, int, int);
 
 void
 free_entry(entry *e)
@@ -142,38 +143,56 @@ load_entry(FILE *file, void (*error_func)(const char *), struct passwd *pw,
 		if (!strcmp("reboot", cmd)) {
 			e->flags |= WHEN_REBOOT;
 		} else if (!strcmp("yearly", cmd) || !strcmp("annually", cmd)){
-			bit_set(e->minute, 0);
-			bit_set(e->hour, 0);
-			bit_set(e->dom, 0);
-			bit_set(e->month, 0);
-			bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
+			set_element(e->minute, FIRST_MINUTE, LAST_MINUTE,
+			    FIRST_MINUTE);
+			set_element(e->hour, FIRST_HOUR, LAST_HOUR, FIRST_HOUR);
+			set_element(e->dom, FIRST_DOM, LAST_DOM, FIRST_DOM);
+			set_element(e->month, FIRST_MONTH, LAST_MONTH,
+			    FIRST_MONTH);
+			set_range(e->dow, FIRST_DOW, LAST_DOW, FIRST_DOW,
+			    LAST_DOW, 1);
 			e->flags |= DOW_STAR;
 		} else if (!strcmp("monthly", cmd)) {
-			bit_set(e->minute, 0);
-			bit_set(e->hour, 0);
-			bit_set(e->dom, 0);
-			bit_nset(e->month, 0, (LAST_MONTH-FIRST_MONTH+1));
-			bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
+			set_element(e->minute, FIRST_MINUTE, LAST_MINUTE,
+			    FIRST_MINUTE);
+			set_element(e->hour, FIRST_HOUR, LAST_HOUR, FIRST_HOUR);
+			set_element(e->dom, FIRST_DOM, LAST_DOM, FIRST_DOM);
+			set_range(e->month, FIRST_MONTH, LAST_MONTH,
+			    FIRST_MONTH, LAST_MONTH, 1);
+			set_range(e->dow, FIRST_DOW, LAST_DOW, FIRST_DOW,
+			    LAST_DOW, 1);
 			e->flags |= DOW_STAR;
 		} else if (!strcmp("weekly", cmd)) {
-			bit_set(e->minute, 0);
-			bit_set(e->hour, 0);
-			bit_nset(e->dom, 0, (LAST_DOM-FIRST_DOM+1));
-			bit_nset(e->month, 0, (LAST_MONTH-FIRST_MONTH+1));
-			bit_set(e->dow, 0);
+			set_element(e->minute, FIRST_MINUTE, LAST_MINUTE,
+			    FIRST_MINUTE);
+			set_element(e->hour, FIRST_HOUR, LAST_HOUR, FIRST_HOUR);
+			set_range(e->dom, FIRST_DOM, LAST_DOM, FIRST_DOM,
+			    LAST_DOM, 1);
+			set_range(e->month, FIRST_MONTH, LAST_MONTH,
+			    FIRST_MONTH, LAST_MONTH, 1);
+			set_element(e->dow, FIRST_DOW, LAST_DOW, FIRST_DOW);
 			e->flags |= DOW_STAR;
 		} else if (!strcmp("daily", cmd) || !strcmp("midnight", cmd)) {
-			bit_set(e->minute, 0);
-			bit_set(e->hour, 0);
-			bit_nset(e->dom, 0, (LAST_DOM-FIRST_DOM+1));
-			bit_nset(e->month, 0, (LAST_MONTH-FIRST_MONTH+1));
-			bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
+			set_element(e->minute, FIRST_MINUTE, LAST_MINUTE,
+			    FIRST_MINUTE);
+			set_element(e->hour, FIRST_HOUR, LAST_HOUR, FIRST_HOUR);
+			set_range(e->dom, FIRST_DOM, LAST_DOM, FIRST_DOM,
+			    LAST_DOM, 1);
+			set_range(e->month, FIRST_MONTH, LAST_MONTH,
+			    FIRST_MONTH, LAST_MONTH, 1);
+			set_range(e->dow, FIRST_DOW, LAST_DOW, FIRST_DOW,
+			    LAST_DOW, 1);
 		} else if (!strcmp("hourly", cmd)) {
-			bit_set(e->minute, 0);
-			bit_nset(e->hour, 0, (LAST_HOUR-FIRST_HOUR+1));
-			bit_nset(e->dom, 0, (LAST_DOM-FIRST_DOM+1));
-			bit_nset(e->month, 0, (LAST_MONTH-FIRST_MONTH+1));
-			bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
+			set_element(e->minute, FIRST_MINUTE, LAST_MINUTE,
+			    FIRST_MINUTE);
+			set_range(e->hour, FIRST_HOUR, LAST_HOUR, FIRST_HOUR,
+			    LAST_HOUR, 1);
+			set_range(e->dom, FIRST_DOM, LAST_DOM, FIRST_DOM,
+			    LAST_DOM, 1);
+			set_range(e->month, FIRST_MONTH, LAST_MONTH,
+			    FIRST_MONTH, LAST_MONTH, 1);
+			set_range(e->dow, FIRST_DOW, LAST_DOW,
+			    FIRST_DOW, LAST_DOW, 1);
 			e->flags |= HR_STAR;
 		} else {
 			ecode = e_timespec;
@@ -426,7 +445,7 @@ get_list(bitstr_t *bits, int low, int high, const char *names[],
 
 	/* clear the bit string, since the default is 'off'.
 	 */
-	bit_nclear(bits, 0, (high-low+1));
+	bit_nclear(bits, 0, high - low);
 
 	/* process all ranges
 	 */
@@ -458,7 +477,7 @@ get_range(bitstr_t *bits, int low, int high, const char *names[],
 	 * number "-" number ["/" number]
 	 */
 
-	int i, num1, num2, num3, rndstep;
+	int num1, num2, num3, rndstep;
 
 	num1 = low;
 	num2 = high;
@@ -578,11 +597,10 @@ get_range(bitstr_t *bits, int low, int high, const char *names[],
 	 * proposed conceptually by bob@acornrc, syntactically
 	 * designed then implemented by paul vixie).
 	 */
-	for (i = num1;  i <= num2;  i += num3)
-		if (set_element(bits, low, high, i) == EOF) {
-			unget_char(ch, file);
-			return (EOF);
-		}
+	if (set_range(bits, low, high, num1, num2, num3) == EOF) {
+		unget_char(ch, file);
+		return (EOF);
+	}
 
 	return (ch);
 }
@@ -643,7 +661,27 @@ set_element(bitstr_t *bits, int low, int high, int number)
 
 	if (number < low || number > high)
 		return (EOF);
+	number -= low;
+
+	bit_set(bits, number);
+	return (0);
+}
+
+static int
+set_range(bitstr_t *bits, int low, int high, int start, int stop, int step)
+{
+	int i;
+
+	if (start < low || stop > high)
+		return (EOF);
+	start -= low;
+	stop -= low;
 
-	bit_set(bits, (number-low));
+	if (step == 1) {
+		bit_nset(bits, start, stop);
+	} else {
+		for (i = start; i <= stop; i += step)
+			bit_set(bits, i);
+	}
 	return (0);
 }