From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotd protected references
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Wed, 5 Apr 2023 10:36:53 +0200

On Tue, Apr 04, 2023 at 09:23:37PM +0200, Omar Polo wrote:
> here's the diff to free the strings in all code paths.  While here,
> I've noticed that there are a few other places where we forgot to free
> the STRING so fix them as well.
> 
> There's a subtle difference between our functions: conf_new_access_rule
> saves the pointer internally so we only free() in the other code-path,
> the new conf_protect_* instead ends up building their own strings and
> so the caller has to free them.

ok stsp

> diff /home/op/w/gotacl
> commit - 919fc1f4bb6c929dbeaf76d0d29352102022f189
> path + /home/op/w/gotacl
> blob - 44801b6de7df40ed4df2acd8f523a9ccabe7170b
> file + gotd/parse.y
> --- gotd/parse.y
> +++ gotd/parse.y
> @@ -249,36 +249,38 @@ protectflags	: TAG NAMESPACE STRING {
>  protectflags	: TAG NAMESPACE STRING {
>  			if (gotd_proc_id == PROC_GOTD ||
>  			    gotd_proc_id == PROC_REPO_WRITE) {
>  				if (conf_protect_tag_namespace(new_repo, $3)) {
>  					free($3);
>  					YYERROR;
>  				}
>  			}
> +			free($3);
>  		}
>  		| BRANCH NAMESPACE STRING {
>  			if (gotd_proc_id == PROC_GOTD ||
>  			    gotd_proc_id == PROC_REPO_WRITE) {
>  				if (conf_protect_branch_namespace(new_repo,
>  				    $3)) {
>  					free($3);
>  					YYERROR;
>  				}
> -				free($3);
>  			}
> +			free($3);
>  		}
>  		| BRANCH STRING {
>  			if (gotd_proc_id == PROC_GOTD ||
>  			    gotd_proc_id == PROC_REPO_WRITE) {
>  				if (conf_protect_branch(new_repo, $2)) {
>  					free($2);
>  					YYERROR;
>  				}
>  			}
> +			free($2);
>  		}
>  		;
>  
>  repository	: REPOSITORY STRING {
>  			struct gotd_repo *repo;
>  
>  			TAILQ_FOREACH(repo, &gotd->repos, entry) {
>  				if (strcmp(repo->name, $2) == 0) {
> @@ -315,30 +317,33 @@ repoopts1	: PATH STRING {
>  				}
>  			}
>  			free($2);
>  		}
>  		| PERMIT RO STRING {
>  			if (gotd_proc_id == PROC_AUTH) {
>  				conf_new_access_rule(new_repo,
>  				    GOTD_ACCESS_PERMITTED, GOTD_AUTH_READ, $3);
> -			}
> +			} else
> +				free($3);
>  		}
>  		| PERMIT RW STRING {
>  			if (gotd_proc_id == PROC_AUTH) {
>  				conf_new_access_rule(new_repo,
>  				    GOTD_ACCESS_PERMITTED,
>  				    GOTD_AUTH_READ | GOTD_AUTH_WRITE, $3);
> -			}
> +			} else
> +				free($3);
>  		}
>  		| DENY STRING {
>  			if (gotd_proc_id == PROC_AUTH) {
>  				conf_new_access_rule(new_repo,
>  				    GOTD_ACCESS_DENIED, 0, $2);
> -			}
> +			} else
> +				free($2);
>  		}
>  		| protect
>  		;
>  
>  repoopts2	: repoopts2 repoopts1 nl
>  		| repoopts1 optnl
>  		;
>  
>