From a6576769f01325303b11edc3e0cfb05ef382ce56 Mon Sep 17 00:00:00 2001
From: Tomas Mraz <tmraz@fedoraproject.org>
Date: Fri, 15 Mar 2019 10:53:29 +0100
Subject: [PATCH] Fix CVE-2019-9704 and CVE-2019-9705

The users can cause DoS of the crond by loading huge crontab files.
We now allow maximum 1000 environment variables and 1000 crontab entries.
Also the comments and whitespace between the entries and variables
are now limited to 32768 characters.
---
 src/crontab.c | 23 ++++++++++++++++++++++-
 src/entry.c   | 29 +++++++++++++++++------------
 src/env.c     |  1 -
 src/funcs.h   |  2 +-
 src/macros.h  |  3 +++
 src/misc.c    | 17 +++++++++++++----
 src/user.c    | 19 ++++++++++++++++++-
 7 files changed, 74 insertions(+), 20 deletions(-)

Index: cronie-1.4.11/src/crontab.c
===================================================================
--- cronie-1.4.11.orig/src/crontab.c
+++ cronie-1.4.11/src/crontab.c
@@ -756,6 +756,7 @@ static int replace_cmd(void) {
 	time_t now = time(NULL);
 	char **envp;
 	char *safename;
+	int envs = 0, entries = 0;
 
 
 	safename = host_specific_filename("tmp.XXXXXXXXXX", 1);
@@ -835,6 +836,10 @@ static int replace_cmd(void) {
 	}
 
 	while (!CheckErrorCount && !eof) {
+		if (!skip_comments(tmp)) {
+			check_error("too many garbage characters");
+			break;
+		}
 		switch (load_env(envstr, tmp)) {
 		case ERR:
 			/* check for data before the EOF */
@@ -846,14 +851,30 @@ static int replace_cmd(void) {
 			break;
 		case FALSE:
 			e = load_entry(tmp, check_error, pw, envp);
-			if (e)
+			if (e) {
+				++entries;
 				free_entry(e);
+			}
 			break;
 		case TRUE:
+			++envs;
 			break;
 		}
 	}
 	env_free(envp);
+	if (envs > MAX_USER_ENVS) {
+		fprintf(stderr, "More than %d environment variables in crontab file, can't install.\n", MAX_USER_ENVS);
+		fclose(tmp);
+		error = -1;
+		goto done;
+	}
+
+	if (entries > MAX_USER_ENTRIES) {
+		fprintf(stderr, "More than %d entries in crontab file, can't install.\n", MAX_USER_ENTRIES);
+		fclose(tmp);
+		error = -1;
+		goto done;
+	}
 
 	if (CheckErrorCount != 0) {
 		fprintf(stderr, "errors in crontab file, can't install.\n");
Index: cronie-1.4.11/src/entry.c
===================================================================
--- cronie-1.4.11.orig/src/entry.c
+++ cronie-1.4.11/src/entry.c
@@ -93,18 +93,17 @@ entry *load_entry(FILE * file, void (*er
 	 */
 
 	ecode_e ecode = e_none;
-	entry *e;
+	entry *e = NULL;
 	int ch;
 	char cmd[MAX_COMMAND];
 	char envstr[MAX_ENVSTR];
 	char **tenvp;
 	char *p;
 	struct passwd temppw;
+	int i;
 
 	Debug(DPARS, ("load_entry()...about to eat comments\n"));
 
-		skip_comments(file);
-
 	ch = get_char(file);
 	if (ch == EOF)
 		return (NULL);
@@ -115,6 +114,10 @@ entry *load_entry(FILE * file, void (*er
 	 */
 
 	e = (entry *) calloc(sizeof (entry), sizeof (char));
+	if (e == NULL) {
+		ecode = e_memory;
+		goto eof;
+	}
 
 	/* check for '-' as a first character, this option will disable 
 	* writing a syslog message about command getting executed
@@ -410,12 +413,14 @@ entry *load_entry(FILE * file, void (*er
 	return (e);
 
   eof:
-	if (e->envp)
-		env_free(e->envp);
-	free(e->pwd);
-	free(e->cmd);
-	free(e);
-	while (ch != '\n' && !feof(file))
+	if (e) {
+		if (e->envp)
+			env_free(e->envp);
+		free(e->pwd);
+		free(e->cmd);
+		free(e);
+	}
+	for (i = 0; i < MAX_COMMAND && ch != '\n' && !feof(file); i++)
 		ch = get_char(file);
 	if (ecode != e_none && error_func)
 		(*error_func) (ecodes[(int) ecode]);
Index: cronie-1.4.11/src/env.c
===================================================================
--- cronie-1.4.11.orig/src/env.c
+++ cronie-1.4.11/src/env.c
@@ -181,7 +181,6 @@ int load_env(char *envstr, FILE * f) {
 
 	filepos = ftell(f);
 	fileline = LineNumber;
-	skip_comments(f);
 	if (EOF == get_string(envstr, MAX_ENVSTR, f, "\n"))
 		return (ERR);
 
Index: cronie-1.4.11/src/funcs.h
===================================================================
--- cronie-1.4.11.orig/src/funcs.h
+++ cronie-1.4.11/src/funcs.h
@@ -49,7 +49,6 @@ void		set_cron_uid(void),
 		unget_char(int, FILE *),
 		free_entry(entry *),
 		acquire_daemonlock(int),
-		skip_comments(FILE *),
 		log_it(const char *, PID_T, const char *, const char *, int),
 		log_close(void),
 		check_orphans(cron_db *);
@@ -71,6 +70,7 @@ int		load_database(cron_db *),
 		cron_pclose(FILE *),
 		glue_strings(char *, size_t, const char *, const char *, char),
 		strcmp_until(const char *, const char *, char),
+		skip_comments(FILE *),
 		allowed(const char * ,const char * ,const char *),
 		strdtb(char *);
 
Index: cronie-1.4.11/src/macros.h
===================================================================
--- cronie-1.4.11.orig/src/macros.h
+++ cronie-1.4.11/src/macros.h
@@ -58,6 +58,9 @@
 #define	MAX_UNAME	256	/* max length of username  */
 #define	ROOT_UID	0	/* don't change this, it really must be root */
 #define	ROOT_USER	"root"	/* ditto */
+#define MAX_USER_ENVS	1000	/* maximum environment variables in user's crontab */
+#define MAX_USER_ENTRIES 1000	/* maximum crontab entries in user's crontab */
+#define MAX_GARBAGE	32768	/* max num of chars of comments and whitespaces between entries */
 
 				/* NOTE: these correspond to DebugFlagNames,
 				 *	defined below.
Index: cronie-1.4.11/src/misc.c
===================================================================
--- cronie-1.4.11.orig/src/misc.c
+++ cronie-1.4.11/src/misc.c
@@ -413,14 +413,20 @@ int get_string(char *string, int size, F
 
 /* skip_comments(file) : read past comment (if any)
  */
-void skip_comments(FILE * file) {
+int skip_comments(FILE * file) {
 	int ch;
+	int n = 0;
 
 	while (EOF != (ch = get_char(file))) {
 		/* ch is now the first character of a line.
 		 */
-		while (ch == ' ' || ch == '\t')
+		if (++n > MAX_GARBAGE)
+			return FALSE;
+		while (ch == ' ' || ch == '\t') {
 			ch = get_char(file);
+			if (++n > MAX_GARBAGE)
+				return FALSE;
+		}
 
 		if (ch == EOF)
 			break;
@@ -435,15 +441,18 @@ void skip_comments(FILE * file) {
 		 * character on a line.
 		 */
 
-		while (ch != '\n' && ch != EOF)
+		while (ch != '\n' && ch != EOF) {
 			ch = get_char(file);
-
+			if (++n > MAX_GARBAGE)
+				return FALSE;
+		}
 		/* ch is now the newline of a line which we're going to
 		 * ignore.
 		 */
 	}
 	if (ch != EOF)
 		unget_char(ch, file);
+	return TRUE;
 }
 
 /* int in_file(const char *string, FILE *file, int error)
Index: cronie-1.4.11/src/user.c
===================================================================
--- cronie-1.4.11.orig/src/user.c
+++ cronie-1.4.11/src/user.c
@@ -65,6 +65,7 @@ load_user (int crontab_fd, struct passwd
 	entry *e;
 	int status = TRUE, save_errno = 0;
 	char **envp = NULL, **tenvp;
+	int envs = 0, entries = 0;
 
 	if (!(file = fdopen(crontab_fd, "r")))	{
 		save_errno = errno;
@@ -89,6 +90,8 @@ load_user (int crontab_fd, struct passwd
 		goto done;
 	}
 
+	u->system = pw == NULL;
+
 	/* init environment.  this will be copied/augmented for each entry.
 	*/
 	if ((envp = env_init()) == NULL) {
@@ -109,9 +112,21 @@ load_user (int crontab_fd, struct passwd
 #endif
 	/* load the crontab
 	*/
-	while ((status = load_env (envstr, file)) >= OK) {
+	while (status >= OK) {
+		if (!skip_comments(file) && !u->system) {
+			log_error("too many garbage characters");
+			status = TRUE;
+			break;
+		}
+		status = load_env (envstr, file);
 		switch (status) {
 			case FALSE:
+				++entries;
+				if (!u->system && entries > MAX_USER_ENTRIES) {
+					log_error("too many entries");
+					status = TRUE;
+					goto done;
+				}
 				FileName = tabname;
 				e = load_entry(file, log_error, pw, envp);
 				if (e) {
@@ -120,6 +135,11 @@ load_user (int crontab_fd, struct passwd
 				}
 				break;
 			case TRUE:
+				++envs;
+				if (!u->system && envs > MAX_USER_ENVS) {
+					log_error("too many environment variables");
+					goto done;
+				}
 				if ((tenvp = env_set (envp, envstr)) == NULL) {
 					save_errno = errno;
 					goto done;
Index: cronie-1.4.11/src/structs.h
===================================================================
--- cronie-1.4.11.orig/src/structs.h
+++ cronie-1.4.11/src/structs.h
@@ -67,6 +67,7 @@ typedef	struct _user {
 	time_t		mtime;		/* last modtime of crontab */
 	entry		*crontab;	/* this person's crontab */
 	security_context_t	scontext;    /* SELinux security context */
+	int		system;		/* is it a system crontab */
 } user;
 
 typedef	struct _orphan {
