From: Jan Schreiber <jes@posteo.de>
Subject: Re: acme-client: close json memory leaks
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Tue, 07 Apr 2026 11:57:16 +0000

On Tue, 7 Apr 2026 13:15:55 +0200
Theo Buehler <tb@theobuehler.org> wrote:

> On Tue, Apr 07, 2026 at 11:04:00AM +0000, Jan Schreiber wrote:
> > On Tue, 7 Apr 2026 12:45:49 +0200
> > Theo Buehler <tb@theobuehler.org> wrote:
> >   
> > > Thanks. Yes, that's basically what I had in mind. Two more nits  
> > 
> > Fixed now, thank you for the review!  
> 
> > @@ -488,12 +489,11 @@ err:
> >  int
> >  json_parse_upd_order(struct jsmnn *n, struct order *order)
> >  {
> > -	char	*certificate;
> >  	order->status = json_parse_order_status(n);
> > -	if ((certificate = json_getstr(n, "certificate")) != NULL) {
> > -		if ((order->certificate = strdup(certificate)) == NULL)
> > -			return 0;
> > -	}
> > +
> > +	if ((order->certificate = json_getstr(n, "certificate")) != NULL)
> > +		return 0;  
> 
> The != NULL check is wrong, if anything it should be == NULL. But
> previously an error check would only happen if there actually was a
> "certificate" present (and strdup didn't fail). I think this should be
> 
> 	order->status = json_parse_order_status(n);
> 	order->certificate = json_getstr(n, "certificate");
> 

You mean removing the NULL check entirely? That would also involve removing
the return code check in netproc.c on line 543 and not handling the error.