[PATCH] Initial work on automatically reallocated buffer size for CSD script

Adam Piątyszek ediap at users.sourceforge.net
Fri Feb 12 09:24:55 EST 2010


Hi David,

* David Woodhouse [12.02.2010 01:07]:
> A good start; thanks. Try something like this...

Thanks for your rewrite. Now it works fine with my company VPN gateway.
BTW, I have found one issue in the following chunk, which might be
problematic:

> @@ -653,38 +672,46 @@ int openconnect_obtain_cookie(struct openconnect_info *vpninfo)
>  		} else {
>  			vpninfo->progress(vpninfo, PRG_ERR, "Relative redirect (to '%s') not supported\n",
>  				vpninfo->redirect_url);
> +			free(form_buf);
>  			return -EINVAL;
>  		}
>  	}
>  
>  	if (vpninfo->csd_stuburl) {
>  		/* This is the CSD stub script, which we now need to run */
> -		result = run_csd_script(vpninfo, buf, buflen);
> -		if (result)
> +		result = run_csd_script(vpninfo, form_buf, buflen);
> +		if (result) {
> +			free(form_buf);
>  			return result;
> +		}
>  
>  		/* Now we'll be redirected to the waiturl */
>  		goto retry;
>  	}
> -	if (strncmp(buf, "<?xml", 5)) {
> +	if (strncmp(form_buf, "<?xml", 5)) {
>  		/* Not XML? Perhaps it's HTML with a refresh... */
> -		if (strcasestr(buf, "http-equiv=\"refresh\"")) {
> +		if (strcasestr(form_buf, "http-equiv=\"refresh\"")) {
>  			vpninfo->progress(vpninfo, PRG_INFO, "Refreshing %s after 1 second...\n",
>  					  vpninfo->urlpath);
>  			sleep(1);
>  			goto retry;
>  		}
>  		vpninfo->progress(vpninfo, PRG_ERR, "Unknown response from server\n");
> +		free(form_buf);
>  		return -EINVAL;
>  	}
>  	request_body[0] = 0;
> -	result = parse_xml_response(vpninfo, buf, request_body, sizeof(request_body),
> +	result = parse_xml_response(vpninfo, form_buf, request_body, sizeof(request_body),
>  				    &method, &request_body_type);
> +	free(form_buf);
> +	form_buf = NULL;
> +
>  	if (!result)
>  		goto redirect;
>  
>  	if (result != 2)
>  		return result;
> +
>  	/* A return value of 2 means the XML form indicated
>  	   success. We _should_ have a cookie... */
>  

In the above chunk you are freeing the form_buf, but the next if can go
back to the piece of code (tagged with "redirect:"), which reuses the
form_buf and even might try to free it again. I propose to move move
these "free(form_buf); form_buf = NULL;" lines after the goto statement.

I will modify this patch and push it to my openconnect-csd2 fork on
git.infradead.com.

BR,
/Adam



More information about the openconnect-devel mailing list