Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion libopendmarc/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ check_PROGRAMS = test_tld \
test_dmarc_fetch \
test_xml_parse \
test_parse_to_buf \
test_alignment
test_alignment \
test_arcares
if LIVE_TESTS
#check_PROGRAMS += test_dns_lookup
#test_dns_lookup_SOURCES = test_dns_lookup.c
Expand All @@ -33,6 +34,11 @@ test_xml_parse_SOURCES = test_xml_parse.c

test_alignment_SOURCES = test_alignment.c

test_arcares_SOURCES = test_arcares.c \
$(top_srcdir)/opendmarc/opendmarc-arcares.c \
$(top_srcdir)/opendmarc/opendmarc-arcseal.c
test_arcares_CPPFLAGS = -I$(top_srcdir)/opendmarc -I.. -I../..

TESTS = $(check_PROGRAMS)

EXTRA_DIST = testfiles/effective_tld_names.dat \
Expand Down
241 changes: 241 additions & 0 deletions libopendmarc/tests/test_arcares.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
/*
** Copyright (c) 2025, The Trusted Domain Project.
** All rights reserved.
**
** Unit tests for ARC-Authentication-Results and ARC-Seal header parsing.
**
** Regression coverage:
** #183/#236 - SIGSEGV: strip_whitespace returned NULL for tokens >= 512
** bytes (e.g. 3072-bit RSA signatures), passed to strlcpy()
** #222/#186 - SIGABRT: malformed tokens with no '=' caused strip_whitespace
** to be called with NULL, hitting assert(string != NULL)
** #241/#242 - memory leaks and NULL dereferences in ARC header parsing
*/

#include "build-config.h"

#include <stdio.h>
#include <string.h>
#include <sys/types.h>

#ifdef HAVE_STDBOOL_H
# include <stdbool.h>
#endif

#ifdef USE_BSD_H
# include <bsd/string.h>
#endif

#ifdef USE_DMARCSTRL_H
# include <opendmarc_strl.h>
#endif

#include "opendmarc-arcares.h"
#include "opendmarc-arcseal.h"

static int pass = 0, fail = 0;

#define CHECK(desc, expr) do { \
if (expr) { \
pass++; \
} else { \
fprintf(stderr, "FAIL: %s\n", (desc)); \
fail++; \
} \
} while (0)

static void
repeat_char(char *buf, char c, size_t n)
{
memset(buf, c, n);
buf[n] = '\0';
}

int
main(int argc, char **argv)
{
struct arcares aar;
struct arcares_arc_field arc_field;
struct arcseal as;
/* 520 'a's: exceeds the old 512-byte token limit */
char longval[520 + 1];
char hdr[4200];
int ret;

repeat_char(longval, 'a', 520);

/* ------------------------------------------------------------------ */
/* opendmarc_arcares_parse: valid header */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcares_parse(
(u_char *)"i=1; example.com; arc=pass; "
"dmarc=pass header.from=example.com; "
"dkim=pass header.d=example.com; "
"spf=pass smtp.mailfrom=example.com",
&aar);
CHECK("arcares_parse valid: returns 0", ret == 0);
CHECK("arcares_parse valid: instance == 1", aar.instance == 1);
CHECK("arcares_parse valid: authserv_id",
strcmp((char *)aar.authserv_id, "example.com") == 0);

/* ------------------------------------------------------------------ */
/* arcares_parse: token with no '=' — SIGABRT regression (#222/#186) */
/* */
/* tag_value = token_ptr where token_ptr is NULL after strsep finds */
/* no '='. atoi(NULL)/snprintf with NULL → crash before the fix. */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcares_parse((u_char *)"noequals", &aar);
CHECK("arcares_parse missing '=': returns -1", ret == -1);

/* ------------------------------------------------------------------ */
/* arcares_parse: overlong authserv_id — SIGSEGV regression (#242) */
/* */
/* strip_whitespace returned NULL for tokens >= 512 bytes; before the */
/* fix strlcpy(authserv_id, NULL, ...) crashed. After the fix the */
/* parse succeeds and the value is truncated to fit the struct field. */
/* ------------------------------------------------------------------ */

snprintf(hdr, sizeof hdr, "i=1; %s; arc=pass", longval);
ret = opendmarc_arcares_parse((u_char *)hdr, &aar);
CHECK("arcares_parse overlong authserv_id: returns 0", ret == 0);
CHECK("arcares_parse overlong authserv_id: instance set", aar.instance == 1);
CHECK("arcares_parse overlong authserv_id: value stored",
strlen((char *)aar.authserv_id) > 0);

/* ------------------------------------------------------------------ */
/* opendmarc_arcares_arc_parse: valid field */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcares_arc_parse(
(u_char *)"arc=pass; smtp.client-ip=1.2.3.4; arc.chain=example.com",
&arc_field);
CHECK("arcares_arc_parse valid: returns 0", ret == 0);
CHECK("arcares_arc_parse valid: arcresult",
strcmp((char *)arc_field.arcresult, "pass") == 0);
CHECK("arcares_arc_parse valid: smtpclientip",
strcmp((char *)arc_field.smtpclientip, "1.2.3.4") == 0);
CHECK("arcares_arc_parse valid: arcchain",
strcmp((char *)arc_field.arcchain, "example.com") == 0);

/* ------------------------------------------------------------------ */
/* arcares_arc_parse: token with no '=' — SIGABRT regression (#222) */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcares_arc_parse((u_char *)"noequals", &arc_field);
CHECK("arcares_arc_parse missing '=': returns -1", ret == -1);

/* ------------------------------------------------------------------ */
/* arcares_arc_parse: overlong value — SIGSEGV regression (#242) */
/* */
/* After the fix the parse succeeds; value truncated to field size. */
/* ------------------------------------------------------------------ */

snprintf(hdr, sizeof hdr, "arc=%s", longval);
ret = opendmarc_arcares_arc_parse((u_char *)hdr, &arc_field);
CHECK("arcares_arc_parse overlong value: returns 0", ret == 0);
CHECK("arcares_arc_parse overlong value: arcresult stored",
strlen((char *)arc_field.arcresult) > 0);

/* ------------------------------------------------------------------ */
/* opendmarc_arcseal_parse: valid header */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcseal_parse(
(u_char *)"i=1; a=rsa-sha256; cv=pass; d=example.com; "
"s=sel1; t=1234567890; b=abc123",
&as);
CHECK("arcseal_parse valid: returns 0", ret == 0);
CHECK("arcseal_parse valid: instance == 1", as.instance == 1);
CHECK("arcseal_parse valid: domain",
strcmp((char *)as.signature_domain, "example.com") == 0);
CHECK("arcseal_parse valid: selector",
strcmp((char *)as.signature_selector, "sel1") == 0);
CHECK("arcseal_parse valid: cv",
strcmp((char *)as.chain_validation, "pass") == 0);

/* ------------------------------------------------------------------ */
/* arcseal_parse: token with no '=' — SIGABRT regression (#222/#186) */
/* */
/* "ARC-Seal: i=1; none" style malformed headers previously triggered */
/* assert(string != NULL) inside strip_whitespace → SIGABRT. */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcseal_parse((u_char *)"i=1; none", &as);
CHECK("arcseal_parse token with no '=': returns -1", ret == -1);

/* ------------------------------------------------------------------ */
/* arcseal_parse: long b= value — SIGSEGV regression (#183/#236) */
/* */
/* RSA 3072-bit signatures produce ~512-byte base64 values, hitting */
/* the old MAX_TOKEN_LEN limit. strip_whitespace returned NULL which */
/* was then passed to strlcpy() → segfault. */
/* After the fix the parse succeeds and the value is stored. */
/* ------------------------------------------------------------------ */

snprintf(hdr, sizeof hdr,
"i=1; a=rsa-sha256; cv=pass; d=example.com; s=sel1; "
"t=1234567890; b=%s",
longval);
ret = opendmarc_arcseal_parse((u_char *)hdr, &as);
CHECK("arcseal_parse long b= (RSA 3072+): returns 0", ret == 0);
CHECK("arcseal_parse long b=: instance set", as.instance == 1);
CHECK("arcseal_parse long b=: signature_value stored",
as.signature_value[0] != '\0');
CHECK("arcseal_parse long b=: other fields preserved",
strcmp((char *)as.signature_domain, "example.com") == 0);

/* ------------------------------------------------------------------ */
/* arcseal_parse: overlong non-signature field — truncation check */
/* ------------------------------------------------------------------ */

snprintf(hdr, sizeof hdr, "i=1; d=%s", longval);
ret = opendmarc_arcseal_parse((u_char *)hdr, &as);
CHECK("arcseal_parse overlong d= field: returns 0", ret == 0);
CHECK("arcseal_parse overlong d= field: instance set", as.instance == 1);
CHECK("arcseal_parse overlong d= field: domain stored",
strlen((char *)as.signature_domain) > 0);

/* ------------------------------------------------------------------ */
/* arcares_parse: unknown auth method — real-world regression (#238) */
/* */
/* Gmail ARC headers include "dara=pass" (DKIM Authorized Resigners). */
/* Before the fix, any unknown method in the top-level AAR parser set */
/* result = -1, causing the entire header to be rejected. Known fields*/
/* like dkim=, spf=, dmarc= were silently discarded. */
/* After the fix, unknown methods are skipped; known fields are kept. */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcares_parse(
(u_char *)"i=1; mx.google.com; "
"dkim=pass header.i=@gmail.com; "
"spf=pass smtp.mailfrom=foo@gmail.com; "
"dmarc=pass header.from=gmail.com; "
"dara=pass header.i=@gmail.com",
&aar);
CHECK("arcares_parse unknown method (dara): returns 0", ret == 0);
CHECK("arcares_parse unknown method (dara): instance set", aar.instance == 1);
CHECK("arcares_parse unknown method (dara): dkim stored",
strlen((char *)aar.dkim) > 0);
CHECK("arcares_parse unknown method (dara): dmarc stored",
strlen((char *)aar.dmarc) > 0);

/* ------------------------------------------------------------------ */
/* arcares_parse: CRLF folding (\r\n) in header — regression (#238) */
/* */
/* Multi-line headers have \r\n before continuation whitespace. */
/* strspn used " \n\t" but omitted \r, leaving a stray \r in tokens. */
/* ------------------------------------------------------------------ */

ret = opendmarc_arcares_parse(
(u_char *)"i=1; mx.google.com;\r\n\tdkim=pass header.i=@gmail.com;\r\n\tdmarc=pass header.from=gmail.com",
&aar);
CHECK("arcares_parse CRLF folding: returns 0", ret == 0);
CHECK("arcares_parse CRLF folding: instance set", aar.instance == 1);
CHECK("arcares_parse CRLF folding: dkim stored",
strlen((char *)aar.dkim) > 0);

printf("ARC header parsing: pass=%d, fail=%d\n", pass, fail);
return fail;
}
34 changes: 13 additions & 21 deletions opendmarc/opendmarc-arcares.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@
#endif /* USE_DMARCSTRL_H */

#include "opendmarc-arcares.h"
#include "opendmarc.h"

#define OPENDMARC_ARCARES_MAX_FIELD_NAME_LEN 255
#define OPENDMARC_ARCARES_MAX_TOKEN_LEN 512

#ifndef MAX
# define MAX(x, y) ((x) >= (y)) ? (x) : (y)
Expand Down Expand Up @@ -92,15 +90,13 @@ opendmarc_arcares_convert(struct opendmarc_arcares_lookup *table, char *str)

/*
** OPENDMARC_ARCARES_STRIP_WHITESPACE -- removes all whitespace from a string
** in-place, handling a maximum string of length
** ARCARES_MAX_TOKEN_LEN
** in-place
**
** Parameters:
** string -- NULL-terminated string to modify
**
** Returns:
** pointer to string on success, NULL on failure (max string length
** exceeded)
** pointer to string
**/

static char *
Expand All @@ -110,13 +106,8 @@ opendmarc_arcares_strip_whitespace(u_char *string)

int a;
int b;
char *string_ptr;

string_ptr = string;

for (a = 0, b = 0;
string[b] != '\0' && b < OPENDMARC_ARCARES_MAX_TOKEN_LEN;
b++)
for (a = 0, b = 0; string[b] != '\0'; b++)
{
if (isascii(string[b]) && isspace(string[b]))
continue;
Expand All @@ -125,9 +116,6 @@ opendmarc_arcares_strip_whitespace(u_char *string)
a++;
}

if (b >= OPENDMARC_ARCARES_MAX_TOKEN_LEN)
return NULL;

/* set remaining chars to null */
memset(&string[a], '\0', b - a);

Expand Down Expand Up @@ -174,11 +162,13 @@ opendmarc_arcares_parse (u_char *hdr, struct arcares *aar)
char *tag_label;
char *tag_value;

leading_space_len = strspn(token, " \n\t");
leading_space_len = strspn(token, " \r\n\t");
token_ptr = token + leading_space_len;
if (*token_ptr == '\0')
return 0;
tag_label = strsep(&token_ptr, "=");
if (token_ptr == NULL)
return -1;
tag_value = token_ptr;
tag_code = opendmarc_arcares_convert(aar_tags, tag_label);

Expand All @@ -201,8 +191,10 @@ opendmarc_arcares_parse (u_char *hdr, struct arcares *aar)
/* next value will be unlabeled authserv_id */
if ((token = strsep((char **) &tmp_ptr, ";")) != NULL)
{
leading_space_len = strspn(token, " \n\t");
leading_space_len = strspn(token, " \r\n\t");
tag_value = opendmarc_arcares_strip_whitespace(token);
if (tag_value == NULL)
return -1;
strlcpy(aar->authserv_id, tag_value, sizeof aar->authserv_id);
}
break;
Expand All @@ -212,7 +204,6 @@ opendmarc_arcares_parse (u_char *hdr, struct arcares *aar)
break;

default:
result = -1;
break;
}
}
Expand Down Expand Up @@ -251,7 +242,7 @@ opendmarc_arcares_arc_parse (u_char *hdr_arc, struct arcares_arc_field *arc)
memset(arc, '\0', sizeof *arc);
memset(tmp, '\0', sizeof tmp);

memcpy(tmp, hdr_arc, MIN_OF(strlen(hdr_arc), sizeof tmp - 1));
memcpy(tmp, hdr_arc, MIN(strlen(hdr_arc), sizeof tmp - 1));

while ((token = strsep((char **)&tmp_ptr, ";")) != NULL)
{
Expand All @@ -261,11 +252,13 @@ opendmarc_arcares_arc_parse (u_char *hdr_arc, struct arcares_arc_field *arc)
char *tag_label;
char *tag_value;

leading_space_len = strspn(token, " \n\t");
leading_space_len = strspn(token, " \r\n\t");
token_ptr = token + leading_space_len;
if (*token_ptr == '\0')
return 0;
tag_label = strsep(&token_ptr, "=");
if (token_ptr == NULL)
return -1;
tag_value = opendmarc_arcares_strip_whitespace(token_ptr);
tag_code = opendmarc_arcares_convert(aar_arc_tags, tag_label);

Expand All @@ -284,7 +277,6 @@ opendmarc_arcares_arc_parse (u_char *hdr_arc, struct arcares_arc_field *arc)
break;

default:
result = -1;
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion opendmarc/opendmarc-arcares.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
/* max header tag value length (short) */
#define OPENDMARC_ARCARES_MAX_SHORT_VALUE_LEN 256
/* max header tag value length (long) */
#define OPENDMARC_ARCARES_MAX_LONG_VALUE_LEN 512
#define OPENDMARC_ARCARES_MAX_LONG_VALUE_LEN 2048

/* names and field labels */
#define OPENDMARC_ARCARES_HDRNAME "ARC-Authentication-Results"
Expand Down
Loading