Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: rpki-client: avoid crash in file mode
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Thu, 6 Jun 2024 09:11:54 +0200

Download raw body.

Thread
On Thu, Jun 06, 2024 at 08:03:47AM +0200, Theo Buehler wrote:
> On Thu, Jun 06, 2024 at 07:57:53AM +0200, Claudio Jeker wrote:
> > On Thu, Jun 06, 2024 at 07:44:59AM +0200, Theo Buehler wrote:
> > > Looking at an expired TA cert crashes because ta_parse() frees cert
> > > while expires and notafter still point at members of that struct.
> > > Since expires != NULL, we'll try to cert_print(NULL) and boom.
> 
> > It seems other code only clears expires but not notafter. Should those
> > bits also be adjusted and clear notafter as well?
> 
> Right, I noticed the one in the other branch right after sending. I
> think I prefer to invalidate and reset after we know they're good.
> 
> Can't spot more than this one.

Like this better. OK claudio@
 
> Index: filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> diff -u -p -r1.42 filemode.c
> --- filemode.c	20 May 2024 15:51:43 -0000	1.42
> +++ filemode.c	6 Jun 2024 06:01:04 -0000
> @@ -526,9 +526,15 @@ proc_parser_file(char *file, unsigned ch
>  			constraints_validate(file, cert);
>  		}
>  	} else if (is_ta) {
> +		expires = NULL;
> +		notafter = NULL;
>  		if ((tal = find_tal(cert)) != NULL) {
>  			cert = ta_parse(file, cert, tal->pkey, tal->pkeysz);
>  			status = (cert != NULL);
> +			if (status) {
> +				expires = &cert->expires;
> +				notafter = &cert->notafter;
> +			}
>  			if (outformats & FORMAT_JSON)
>  				json_do_string("tal", tal->descr);
>  			else
> @@ -538,7 +544,6 @@ proc_parser_file(char *file, unsigned ch
>  		} else {
>  			cert_free(cert);
>  			cert = NULL;
> -			expires = NULL;
>  			status = 0;
>  		}
>  	}
> 

-- 
:wq Claudio