On Mon, May 09, 2011 at 08:51:01PM -0400, Lawrence Teo wrote:
> 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?
Have to look in detail for the stuctural changes, but free(NULL) has
ben OK for ages.
-Otto
>
> 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)
