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

[Peer, Ilan]

Hi Jouni,

> -----Original Message-----
> From: Jouni Malinen [mailto:j at w1.fi]
> Sent: Friday, June 19, 2015 01:22
> To: Peer, Ilan
> Cc: hostap at lists.shmoo.com; Rosenfeld, Ben
> Subject: Re: [PATCH 5/5] WPS: Fix possible memory leak in
> wps_er_config_token_from_cred()
> 
> On Wed, Jun 17, 2015 at 04:16:36PM +0300, Ilan Peer 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);
> >  		wpabuf_free(ret);
> >  		return NULL;
> >  	}
> >
> > +	os_free(data.new_psk);
> 
> Could you please clarify how data.new_psk could be allocated on this code
> path? data.use_cred is used to skip new credential allocation in
> wps_build_cred(), i.e., all the cases that could allocate new_psk are skipped
> with "goto use_provided".
> 

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_registrar.c:1686: wps->auth_type& (2|32) is true
wps_registrar.c:1691: 'wps->new_psk' is allocated by function 'malloc'.
wps_registrar.c:1692: wps->new_psk== ( (void* )0) is false
wps_registrar.c:1694: random_pool_ready() !=1||random_get_bytes(wps->new_psk, wps->new_psk_len) <0 is false
wps_er.c:2040: wps_build_cred( &data, ret) ||wps_build_wfa_ext(ret, 0, ( (void* )0), 0) is true
wps_er.c:2043: Dynamic memory stored in 'data.new_psk' is lost.


Regards,

Ilan.