Skip to content

Abandon ARC header parsing on unexpected data#223

Closed
abokth wants to merge 2 commits into
trusteddomainproject:developfrom
abokth:arc-parsefix-develop
Closed

Abandon ARC header parsing on unexpected data#223
abokth wants to merge 2 commits into
trusteddomainproject:developfrom
abokth:arc-parsefix-develop

Conversation

@abokth
Copy link
Copy Markdown

@abokth abokth commented Jul 6, 2022

This should solve the crash in #183 (but not actually parse the headers).

(patch not written by me)

manu0401 and others added 2 commits February 17, 2021 02:54
@thegushi
Copy link
Copy Markdown
Collaborator

Thank you for this fix — the NULL guards you added are exactly the right approach for the SIGABRT crash path, and this PR correctly diagnosed the missing = case.

A few gaps remained that PR #296 addresses:

  1. The guards in this PR check token_ptr == NULL after strsep(), which handles the missing = case. However, the underlying cause of the SIGSEGV (large RSA key signatures producing tokens longer than 512 bytes) was still present — strip_whitespace() still returned NULL for those, which would then be passed to strlcpy(). PR Fix ARC header parsing crashes: SIGABRT on malformed tokens and SIGSEGV on large RSA key signatures #296 removes the artificial 512-byte token length cap entirely.

  2. The opendmarc_arcares_parse function (the first parser, not the _arc_parse variant) also had an unguarded tag_value = token_ptr path where a missing = would result in NULL being passed to atoi() and snprintf(). That function was not covered here.

  3. Struct field sizes in both .h files were still 512 bytes, which would cause truncation for large-key signatures even after the crash was prevented.

Closing in favour of #296, which addresses all of the above. Thanks again for the contribution!

@thegushi thegushi closed this May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants