Package: cron
Version: 3.0pl1-212
Severity: normal
Tags: patch
While scanning /etc/cron.d, cron prefixes each accepted filename with
"*system*" in a fixed-size fname buffer using strcpy()/strcat(). A
valid maximum-length directory entry can exceed the remaining buffer
capacity and overwrite stack memory. On the usual filesystem this
requires creating a long name in the root-controlled /etc/cron.d
directory, so this is a hardening and reliability issue; no
unprivileged control or privilege escalation was demonstrated.
The attached Debian -212 patch uses checked snprintf() construction
for both the prefixed internal name and the path, and skips an entry
that does not fit. Its regression test covers exact accepted and
first-rejected filename/path boundaries.
The patched Debian source built in a disposable trixie container; the
focused regression and all 13 existing Debian integration tests
passed. Ubuntu versions 3.0pl1-200ubuntu1 and -209ubuntu1 have the
same issue. Debian and Ubuntu Security Teams recommended public bug
reports.
Regards,
Brett Sutton
Description: Bound construction of system crontab names and paths
Origin: other, private Daybreak audit
Forwarded: no
--- a/cron.h
+++ b/cron.h
@@ -347,3 +347,5 @@
#endif /*MAIN_PROGRAM*/
int cron_gethostname(char *, size_t);
+
+int cron_system_name(char *, size_t, char *, size_t, const char *);
--- a/database.c
+++ b/database.c
@@ -190,9 +190,11 @@
continue;
/* Generate the "fname" */
- (void) strcpy(fname,"*system*");
- (void) strcat(fname, dp->d_name);
- sprintf(tabname,"%s/%s", SYSCRONDIR, dp->d_name);
+ if (!cron_system_name(fname, sizeof fname, tabname,
+ sizeof tabname, dp->d_name)) {
+ log_it("CRON", getpid(), "NAME TOO LONG", dp->d_name);
+ continue;
+ }
/* statbuf is used as working storage by process_crontab() --
current contents are irrelevant */
--- a/misc.c
+++ b/misc.c
@@ -802,4 +802,15 @@
(void) strcpy(hostname, "localhost");
hostname[size - 1] = '\0';
return TRUE;
-}
\ No newline at end of file
+}
+/* Reject names that cannot fit both the internal marker and the path. */
+int
+cron_system_name(char *name, size_t name_size, char *path, size_t path_size,
+ const char *filename)
+{
+ int name_len = snprintf(name, name_size, "*system*%s", filename);
+ int path_len = snprintf(path, path_size, "%s/%s", SYSCRONDIR, filename);
+ return name_len >= 0 && path_len >= 0 &&
+ (size_t)name_len < name_size && (size_t)path_len < path_size;
+}
+
--- /dev/null
+++ b/tests/system_name_regression.c
@@ -0,0 +1,17 @@
+#define MAIN_PROGRAM
+#include "../misc.c"
+#include <assert.h>
+int main(void) {
+ char name[MAXNAMLEN + 1], path[PATH_MAX + 1], filename[MAXNAMLEN + 1];
+ size_t accepted = sizeof name - sizeof "*system*";
+ memset(filename, 'a', sizeof filename);
+ filename[accepted + 1] = '\0';
+ assert(!cron_system_name(name, sizeof name, path, sizeof path, filename));
+ filename[accepted] = '\0';
+ assert(cron_system_name(name, sizeof name, path, sizeof path, filename));
+ assert(strlen(name) == sizeof name - 1);
+ assert(strlen(path) == strlen(SYSCRONDIR) + 1 + accepted);
+ assert(!cron_system_name(name, sizeof name, path, strlen(SYSCRONDIR) + 1 + accepted, filename));
+ assert(cron_system_name(name, sizeof name, path, strlen(SYSCRONDIR) + 2 + accepted, filename));
+ return 0;
+}
