On Tue, May 10, 2011 at 06:57:29AM +0200, Otto Moerbeek wrote:
> 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
>
Ditto. I think the calloc() return value check is good as e is referenced
in following code. But free_entry should be ok if you ensure you don't
pass it NULL.
.... Ken
> >
> > 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)
