From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: plug leak regarding repo_commit (plus bonus)
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 1 Sep 2022 13:07:39 +0200

On Thu, Sep 01, 2022 at 12:56:22PM +0200, Omar Polo wrote:
> I missed that tag_name is advanced in a few places, so now it can die
> in gotweb_free_repo_tag:
> 
> gotwebd(30578) in free(): modified chunk-pointer 0xb1f47deaa45
> 
> I'll populate my local /var/www/got/public with more interesting
> repositories to avoid issues like this in the future, sorry.
> 
> is this ok-ish or need to introduce an ad-hoc temp variable?

Looks fine to me. ok stsp
 
> diff /home/op/w/got
> commit - 625e5896fc9ecf87ccfc92ad2a65cd3be58f73c0
> path + /home/op/w/got
> blob - 0feafb3e945e44bdf1ef9bc88c821630cfd8f703
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -1693,9 +1693,10 @@ gotweb_render_tag(struct request *c)
>  	if (error)
>  		goto done;
>  
> -	if (strncmp(rt->tag_name, "refs/", 5) == 0)
> -		rt->tag_name += 5;
> -	error = gotweb_escape_html(&tagname, rt->tag_name);
> +	tagname = rt->tag_name;
> +	if (strncmp(tagname, "refs/", 5) == 0)
> +		tagname += 5;
> +	error = gotweb_escape_html(&tagname, tagname);
>  	if (error)
>  		goto done;
>  
> @@ -1780,9 +1781,10 @@ gotweb_render_tags(struct request *c)
>  		if (error)
>  			goto done;
>  
> -		if (strncmp(rt->tag_name, "refs/tags/", 10) == 0)
> -			rt->tag_name += 10;
> -		error = gotweb_escape_html(&tagname, rt->tag_name);
> +		tagname = rt->tag_name;
> +		if (strncmp(tagname, "refs/tags/", 10) == 0)
> +			tagname += 10;
> +		error = gotweb_escape_html(&tagname, tagname);
>  		if (error)
>  			goto done;
>  
>