[PATCH 5/5] WPS: Fix possible memory leak in wps_er_config_token_from_cred()

[Peer, Ilan]

> -----Original Message-----
> From: Jouni Malinen [mailto:j at w1.fi]
> Sent: Monday, June 29, 2015 20:42
> To: Peer, Ilan
> Cc: Rosenfeld, Ben; hostap at lists.shmoo.com
> Subject: Re: [PATCH 5/5] WPS: Fix possible memory leak in
> wps_er_config_token_from_cred()
> 
> On Sun, Jun 21, 2015 at 01:56:45PM +0000, Peer, Ilan wrote:
> > > > In wps_er_config_token_from_cred() data.new_pak memory is
> > > > allocated in
> > > > wps_build_cred() and the function returns before the memroy is
> released.
> > >
> > > > diff --git a/src/wps/wps_er.c b/src/wps/wps_er.c @@ -2039,10
> > > > +2039,12 @@ struct wpabuf * wps_er_config_token_from_cred(struct
> > > > wps_context
> > > *wps,
> > > >  	data.use_cred = cred;
> > > >  	if (wps_build_cred(&data, ret) ||
> > > >  	    wps_build_wfa_ext(ret, 0, NULL, 0)) {
> > > > +		os_free(data.new_psk);
> 
> > This is the traceback from the tool:
> >
> > wps_er.c:2040: Dynamic memory stored in 'data.new_psk' is allocated by
> calling function 'wps_build_cred'.
> 
> wps_er.c:2039 sets data.user_cred to a non-NULL value (neither of the two
> callers of wps_er_config_token_from_cred() can use NULL as the cred
> argument).
> 
> > wps_registrar.c:1686: wps->auth_type& (2|32) is true
> 
> this is within wps_build_cred() after these steps:
> 
>     if (wps->use_cred) {
> 	os_memcpy(&wps->cred, wps->use_cred, sizeof(wps->cred));
> 	goto use_provided;
>     }
> 
> and before the use_provided label. There is no label in the middle either, so
> no way to get back to line 1686. In other words, this code path is not possible.
> 

Thanks for the clarification. Please drop this patch.

Ilan.