SIGSEGV in arc_chain_custody_str() on an incomplete ARC set when PermitAuthenticationOverrides is enabled

[loyderhy2]

What happens

arc_chain_custody_str() dereferences arcset_ams without checking it, so a message whose ARC chain has an instance with an ARC-Authentication-Results header but no ARC-Message-Signature crashes the milter with SIGSEGV — but only when PermitAuthenticationOverrides is enabled.

#0  arc_chain_custody_str (msg=..., buf=..., buflen=4097) at libopenarc/arc.c:3914
#1  mlfi_eom (ctx=...) at openarc/openarc.c:3942

kvset = <error reading variable: Cannot access memory at address 0x20>
set = 1,  msg->arc_nsets = 2,  msg->arc_cstate = 2 (ARC_CHAIN_PASS)
msg->arc_sets[0] = {arcset_aar = 0x…3740, arcset_ams = 0x…4470, arcset_as = 0x…68d0}
msg->arc_sets[1] = {arcset_aar = 0x…db20, arcset_ams = 0x0,     arcset_as = 0x0}

hdr_data sits at offset 0x20, which is the segfault at 20 the kernel logs.

Why the existing guard does not catch it

libopenarc already knows this shape. arc_eoh() walks the sets, logs missing or incomplete ARC set at instance %u and sets arc_cstate = ARC_CHAIN_FAIL; arc_eom() opens with "nothing to do if the chain has been expressly failed" and returns. That early return is also what keeps arc_validate_msg() safe, since it dereferences arcset_ams unguarded too (arc.c:2243). The invariant is cstate == ARC_CHAIN_PASS implies every set is complete.

PermitAuthenticationOverrides breaks that invariant from outside the library. mlfi_eom() takes the ARC result from a prior Authentication-Results bearing the configured authserv-id and installs it with arc_set_cv() — after arc_eom() has already returned. arc_set_cv() honours only msg->arc_infail, which the structural check never sets, so FAIL becomes PASS. It also promotes none to pass whenever the message carries any sets at all, so a prior arc=none is enough to trigger this.

arc_chain_custody_str()'s only guard is if (msg->arc_cstate != ARC_CHAIN_PASS) return 0;, which now passes, and the loop walks into the NULL.

Reproduction

Two ARC sets, the second one AAR-only, plus a prior Authentication-Results carrying the same authserv-id as the milter:

Authentication-Results: mx1.example.net; dkim=pass header.d=example.com; spf=pass; arc=none
ARC-Authentication-Results: i=1; relay.example.net; spf=pass smtp.mailfrom=sender@example.com
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=relay.example.net; s=arc; …
ARC-Seal: i=1; a=rsa-sha256; d=relay.example.net; s=arc; cv=none; b=…
ARC-Authentication-Results: i=2; gateway.example.org; spf=pass smtp.mailfrom=sender@example.com

Run through 1.3.0 in Mode sv with AuthservID mx1.example.net. The matrix is one cell wide:

case	PermitAuthenticationOverrides true	false (default)
both sets complete	alive	alive
incomplete set, no prior A-R	alive	alive
incomplete set + prior A-R	SIGSEGV	alive

It needs both halves: the incomplete set alone is caught by arc_eoh(), and the prior result alone is harmless.

How we hit it

Real inbound mail, not a crafted message. A commercial mail gateway added an ARC-Authentication-Results: i=2 and neither of the other two headers of the set. Our milter runs behind rspamd, which writes an Authentication-Results and defaults to the same authserv-id OpenARC does, so the "prior result" was our own. milter_default_action = accept meant mail kept flowing unsealed rather than being deferred, so the only symptom was the restart counter.

Suggested fix

Guarding the dereference in arc_chain_custody_str() is the smallest change and is what we are carrying:

for (set = msg->arc_nsets - 1; set >= 0; set--)
{
    if (msg->arc_sets[set].arcset_ams == NULL ||
        msg->arc_sets[set].arcset_ams->hdr_data == NULL)
    {
        arc_dstring_free(tmpbuf);
        return 0;
    }

    kvset = msg->arc_sets[set].arcset_ams->hdr_data;
    str = arc_param_get(kvset, "d");
    if (str == NULL)
    {
        arc_dstring_free(tmpbuf);
        return 0;
    }

    (void) arc_dstring_printf(tmpbuf, "%s%s",
                              (set < msg->arc_nsets - 1 ? ":" : ""), str);
}

With it, upstream's own suite still reports 54 passed / 6 skipped, and an 8-shape × 2-configuration milter matrix is byte-identical between the stock and patched builds.

Worth considering separately: having arc_set_cv() refuse to lift a structural failure (set msg->arc_infail alongside arc_cstate in arc_eoh()) would restore the invariant itself rather than defending each reader of it — arc_validate_msg() has the same unguarded dereference and is safe only by reachability today.

Version

1.3.0 (d40ae0603f), built from source on Ubuntu 24.04.