File: Recover-from-crontab-errors.patch

package info (click to toggle)
cron 3.0pl1-197
  • links: PTS, VCS
  • area: main
  • in suites: trixie
  • size: 3,816 kB
  • sloc: ansic: 54,879; xml: 1,600; perl: 733; sh: 463; makefile: 446; python: 43
file content (190 lines) | stat: -rw-r--r-- 6,808 bytes parent folder | download | duplicates (2)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
From: Christian Kastner <ckk@kvr.at>
Date: Fri, 15 Jan 2016 22:59:09 +0100
Subject: Recover from crontab errors

The way the current change detection logic notices changes to crontabs, files
with mode or syntax errors, and broken symlinks do not automatically get added
back once the problem is resolved.

This solution forces a rescan at next-wakeup. The symlink recovery is only
relevant to /etc/cron.d because SPOOL_DIR does not allow symlinks.

This solution originally contained an error that was classified as a
security issue with CVE-2019-9706.

Bug-Debian: https://bugs.debian.org/433609
Bug-Debian: https://bugs.debian.org/625495
Bug-Debian: https://bugs.debian.org/627859
Bug-Debian: https://bugs.debian.org/809167
Forwarded: no
Last-Update: 2019-03-23
---
 database.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 3 deletions(-)

diff --git a/database.c b/database.c
index 40317b1..f16be38 100644
--- a/database.c
+++ b/database.c
@@ -27,6 +27,7 @@ static char rcsid[] = "$Id: database.c,v 2.8 1994/01/15 20:43:43 vixie Exp $";
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/file.h>
+#include <time.h>
 
 
 #define TMAX(a,b) ((a)>(b)?(a):(b))
@@ -50,6 +51,8 @@ static	void		process_crontab __P((char *, char *, char *,
 static int valid_name (char *filename);
 static user *get_next_system_crontab __P((user *));
 
+void force_rescan_user(cron_db *old_db, cron_db *new_db, const char *fname, time_t old_mtime);
+
 void
 load_database(old_db)
 	cron_db		*old_db;
@@ -120,7 +123,8 @@ load_database(old_db)
 			if (stat(syscrond_fname, &syscrond_file_stat) < OK)
 				syscrond_file_stat.st_mtime = 0;
 
-			if (syscrond_file_stat.st_mtime != systab->mtime) {
+			if (syscrond_file_stat.st_mtime != systab->mtime ||
+					systab->mtime == 0) {
 				syscrond_change = 1;
                         }
 
@@ -339,6 +343,7 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 
 		if (statbuf->st_uid != pw->pw_uid && statbuf->st_uid != ROOT_UID) {
 			log_it(fname, getpid(), "WRONG FILE OWNER", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 
@@ -351,12 +356,14 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 		/* Check to make sure that the crontab's permissions are secure */
 		if ((statbuf->st_mode & 07777) != 0600) {
 			log_it(fname, getpid(), "INSECURE MODE (mode 0600 expected)", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 
 		/* Check to make sure that there are no hardlinks to the crontab */
 		if (statbuf->st_nlink != 1) {
 			log_it(fname, getpid(), "NUMBER OF HARD LINKS > 1", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 	} else {
@@ -368,12 +375,24 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 		}
 		if (S_ISLNK(statbuf->st_mode) && statbuf->st_uid != ROOT_UID) {
 			log_it(fname, getpid(), "WRONG SYMLINK OWNER", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 		if ((crontab_fd = open(tabname, O_RDONLY, 0)) < OK) {
 			/* crontab not accessible?
+			 *
+			 * If tabname is a symlink, it's most probably just
+			 * broken, so we force a rescan. Once the link is
+			 * fixed, it will get picked up and processed again. If
+			 * tabname is a regular file, this error is bad, so we
+			 * skip it instead.
 			 */
-			log_it(fname, getpid(), "CAN'T OPEN", tabname);
+			if (S_ISLNK(statbuf->st_mode)) {
+				log_it(fname, getpid(), "CAN'T OPEN SYMLINK", tabname);
+				force_rescan_user(old_db, new_db, fname, 0);
+			} else {
+				log_it(fname, getpid(), "CAN'T OPEN", tabname);
+			}
 			goto next_crontab;
 		}
 
@@ -385,6 +404,7 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 		/* Check to make sure that the crontab is owned by root */
 		if (statbuf->st_uid != ROOT_UID) {
 			log_it(fname, getpid(), "WRONG FILE OWNER", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 
@@ -400,6 +420,7 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 		 */
 		if ((statbuf->st_mode & S_IWGRP) || (statbuf->st_mode & S_IWOTH)) {
 			log_it(fname, getpid(), "INSECURE MODE (group/other writable)", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 		/* Technically, we should also check whether the parent dir is
@@ -412,6 +433,7 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 		/* Check to make sure that there are no hardlinks to the crontab */
 		if (statbuf->st_nlink != 1) {
 			log_it(fname, getpid(), "NUMBER OF HARD LINKS > 1", tabname);
+			force_rescan_user(old_db, new_db, fname, 0);
 			goto next_crontab;
 		}
 	}
@@ -451,7 +473,15 @@ process_crontab(uname, fname, tabname, statbuf, new_db, old_db)
 	if (u != NULL) {
 		u->mtime = statbuf->st_mtime;
 		link_user(new_db, u);
-	}
+        } else {
+                /* The crontab we attempted to load contains a syntax error. A
+                 * fix won't get picked up by the regular change detection
+                 * code, so we force a rescan. statbuf->st_mtime still contains
+                 * the file's mtime, so we use it to rescan only when an update
+                 * has actually taken place.
+                 */
+                force_rescan_user(old_db, new_db, fname, statbuf->st_mtime);
+        }
 
 next_crontab:
 	if (crontab_fd >= OK) {
@@ -509,3 +539,44 @@ get_next_system_crontab (curtab)
 			break;
 	return curtab;
 }
+
+/* Force rescan of a crontab the next time cron wakes up
+ *
+ * cron currently only detects changes caused by an mtime update; it does not
+ * detect other attribute changes such as UID or mode. To allow cron to recover
+ * from errors of that nature as well, this function removes the crontab from
+ * the old DB (if present there) and adds an empty crontab to the new DB with
+ * a given mtime. Specifying mtime as 0 will force a rescan the next time the
+ * daemon wakes up.
+ */
+void
+force_rescan_user(cron_db *old_db, cron_db *new_db, const char *fname, time_t old_mtime)
+{
+        user *u;
+
+	/* Remove from old DB and free resources */
+	u = find_user(old_db, fname);
+	if (u != NULL) {
+		Debug(DLOAD, (" [delete old data]"))
+		unlink_user(old_db, u);
+		free_user(u);
+	}
+
+	/* Allocate an empty crontab with the specified mtime, add it to new DB */
+	if ((u = (user *) malloc(sizeof(user))) == NULL) {
+		errno = ENOMEM;
+		return;
+	}
+	if ((u->name = strdup(fname)) == NULL) {
+		free(u);
+		errno = ENOMEM;
+		return;
+	}
+	u->mtime = old_mtime;
+	u->crontab = NULL;
+#ifdef WITH_SELINUX
+	u->scontext = NULL;
+#endif
+	Debug(DLOAD, ("\t%s: [added empty placeholder to force rescan]\n", fname))
+	link_user(new_db, u);
+}