From: Jonathan Matthew <jonathan@d14n.org>
Subject: Re: acme-client: fix segfault
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Thu, 17 Sep 2026 07:20:37 +1000

On Mon, Sep 14, 2026 at 08:57:08PM +0200, Jan Klemkow wrote:
> Hi,
> 
> I see a segfault in the netproc process of our acme-client, while
> talking to an OpenXPKI instance:
> 
> acme-client: signal: netproc(18226): Segmentation fault
> 
> In json_parse_challenge() the json_getobj() function returns NULL, which
> is directly used in json_getstr().

RFC 8555 says that challenge objects that include an error object MUST have
status set to 'invalid', but not that an error object must be present if
there's a problem, so this is something we should handle.

> 
> First solution is to check the first argument of json_getstr(), before
> calling it as its done in all other cases json_getstr() is called.
> See the diff below.

This makes sense to me.  We won't be able to display a useful error message
to the user in this case, but there's nothing much we can do.
ok jmatthew

> 
> Second option would be, to check for n == NULL in json_getstr() itself.
> Thus, we could drop redundant NULL checks with in json.c.

I think I'd rather have acme-client be explicit about what elements it
thinks are required in what it's parsing, even if that means a crash or two
when something unexpected happens.

> 
> Opinions?
> 
> OKs?
> 
> bye,
> jan
> 
> Index: json.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/acme-client/json.c,v
> diff -u -p -r1.25 json.c
> --- json.c	22 May 2026 01:53:10 -0000	1.25
> +++ json.c	14 Sep 2026 17:10:16 -0000
> @@ -401,7 +401,8 @@ json_parse_challenge(struct jsmnn *n, st
>  		p->status = json_parse_response(obj);
>  		if (p->status == CHNG_INVALID) {
>  			error = json_getobj(obj, "error");
> -			p->error = json_getstr(error, "detail");
> +			if (error != NULL)
> +				p->error = json_getstr(error, "detail");
>  		}
>  		return p->uri != NULL && p->token != NULL;
>  	}
>