PATCH for Coverity issues

[Ira Cooper]

On Thu, Feb 20, 2014 at 5:30 AM, Volker Lendecke
<Volker.Lendecke at sernet.de>wrote:

> On Thu, Feb 20, 2014 at 03:47:22PM +0530, Santosh Pradhan wrote:
> > >From ab530f08612b57b62f93d9779252a34c68363563 Mon Sep 17 00:00:00 2001
> > From: Santosh Kumar Pradhan <spradhan at redhat.com>
> > Date: Wed, 19 Feb 2014 16:29:52 +0530
> > Subject: [PATCH 1/2] libads: Avoid NULL pointer dereference
> >
> > If ads_do_search_retry() fails, it may deallocate the memory occupied
> > by passed parameter "ads". So, ads_get_attrname_by_guid() would return
> > NULL. If ads_get_extended_right_name_by_guid() gets invoked without
> > validating the "ads", it would end up with NULL-pointer-dereference.
> >
> > FIX:
> > Validate "ads" before calling ads_get_extended_right_name_by_guid().
> >
> > CID: 242078
> >
> > Signed-off-by: Santosh Kumar Pradhan <spradhan at redhat.com>
> > ---
> >  source3/libads/disp_sec.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/source3/libads/disp_sec.c b/source3/libads/disp_sec.c
> > index 7dcdc95..3a6867c 100644
> > --- a/source3/libads/disp_sec.c
> > +++ b/source3/libads/disp_sec.c
> > @@ -105,6 +105,14 @@ static const char
> *ads_interprete_guid_from_object(ADS_STRUCT *ads,
> >               return talloc_asprintf(mem_ctx, "LDAP attribute: \"%s\"",
> ret);
> >       }
> >
> > +     /*
> > +      * ads_get_attrname_by_guid() can return NULL and "ads" being NULL
> > +      * by ads_destroy() in ads_do_search_retry() code path.
> > +      */
> > +     if (!ads) {
> > +             return NULL;
> > +     }
> > +
>
> Can you explain with a few more words how this patch is
> supposed fix the underlying problem? "ads" is a local
> variable in the routine ads_interprete_guid_from_object. C
> passes this value down to the callees like
> ads_do_search_retry, which can free the object "ads" points
> to. The value in ads_interprete_guid_from_object is still
> !=NULL, but pointing to a now-freed memory object. To fix
> this properly, the whole set of APIs surrounding ADS_STRUCT
> need to be restructured to properly deal with it.
>

My bad.

-1 on this patch.

Because we aren't passing pointer to pointer here, the fact that it gets
freed can't get passed to the enclosing scope.  You can't pass the
knowledge up.

Thanks,

-Ira