In the load_entry() function in cron's entry.c, calloc() is called
but its return value is never checked to see if it is NULL,
potentially causing a NULL pointer dereference. Since crontab(1)
shares code with cron, it is affected as well.

The following diff adds the check for NULL, and also moves the
cleanup code that addresses this condition to free_entry() in order
to remove duplicate code.

Any thoughts or comments?

Thanks,
Lawrence


Index: entry.c
===================================================================
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.32
diff -u -p -r1.32 entry.c
--- entry.c     29 Oct 2009 18:56:47 -0000      1.32
+++ entry.c     10 May 2011 00:33:11 -0000
@@ -57,9 +57,12 @@ static int   get_list(bitstr_t *, int, int
 
 void
 free_entry(entry *e) {
-       free(e->cmd);
-       free(e->pwd);
-       env_free(e->envp);
+       if (e->cmd)
+               free(e->cmd);
+       if (e->pwd)
+               free(e->pwd);
+       if (e->envp)
+               env_free(e->envp);
        free(e);
 }
 
@@ -103,6 +106,10 @@ load_entry(FILE *file, void (*error_func
         */
 
        e = (entry *) calloc(sizeof(entry), sizeof(char));
+       if (e == NULL) {
+               ecode = e_memory;
+               goto eof;
+       }
 
        if (ch == '@') {
                /* all of these should be flagged and load-limited; i.e.,
@@ -387,13 +394,8 @@ load_entry(FILE *file, void (*error_func
        return (e);
 
  eof:
-       if (e->envp)
-               env_free(e->envp);
-       if (e->pwd)
-               free(e->pwd);
-       if (e->cmd)
-               free(e->cmd);
-       free(e);
+       if (e)
+               free_entry(e);
        while (ch != '\n' && !feof(file))
                ch = get_char(file);
        if (ecode != e_none && error_func)

Reply via email to