Fix four crash and memory-safety bugs (issues #18, #140, #152, #256)#298
Open
thegushi wants to merge 1 commit into
Open
Fix four crash and memory-safety bugs (issues #18, #140, #152, #256)#298thegushi wants to merge 1 commit into
thegushi wants to merge 1 commit into
Conversation
#256 - opendmarc_policy_fetch_ruf(): || should be && when guarding the memset() call. The condition (list_buf != NULL || size_of_buf > 0) would call memset(NULL, '\0', size) when list_buf is NULL but size_of_buf is non-zero, causing a segfault. Changed to &&, matching the identical guard already used correctly in opendmarc_policy_fetch_rua(). Regression test added to test_dmarc_fetch.c. #18 - dmarcf_config_free(): remove assert(conf->conf_refcnt == 0). Several shutdown and config-reload call sites invoke dmarcf_config_free() without first checking the refcount, so the assert can fire on normal teardown paths, aborting the daemon. #140 - mlfi_envfrom(): replace strncpy(mctx_envdomain, p+1, BUFRSZ) with strlcpy(..., sizeof mctx_envdomain). strncpy does not guarantee null-termination when the source fills the buffer; strlcpy always does. #152 - Bump MAXHEADER from 1024 to 4096 in opendmarc.h. Two snprintf() calls building the Authentication-Results header can produce ~2080 bytes; the old 1025-byte buffer caused silent truncation of outgoing headers. Also fix the off-by-one inconsistency in opendmarc-ar.c (MAXHEADER+2 -> MAXHEADER+1).
thegushi
added
the
needs-testing
This was referenced May 12, 2026
Contributor
|
It has to be evaluated, if the field |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
opendmarc_policy_fetch_ruf():||→&&in the memset guard, matching the correct&&already used inopendmarc_policy_fetch_rua(). Preventsmemset(NULL, …)when called with a NULL buffer and non-zero size. Regression test added totest_dmarc_fetch.c.dmarcf_config_free(): removeassert(conf->conf_refcnt == 0), which fires on normal shutdown and config-reload paths that call the function without checking the refcount first.mlfi_envfrom(): replacestrncpy(mctx_envdomain, p+1, BUFRSZ)withstrlcpy(…, sizeof mctx_envdomain)to guarantee null-termination when the source fills the buffer.MAXHEADERfrom 1024 → 4096 inopendmarc.h; twosnprintf()calls building theAuthentication-Resultsheader can produce ~2080 bytes, causing silent truncation of outgoing headers. Also fixes theMAXHEADER+2→MAXHEADER+1off-by-one inopendmarc-ar.c.Closes #18, #140, #152, #256. Tracked in #297.
Test plan
make checkpasses (7/7)test_dmarc_fetchcoversfetch_rufandfetch_ruawith NULL buf + non-zero size (the This is a null pointer dereference vulnerability occurring at line 1480 in the file /OpenDMARC/libopendmarc/opendmarc_policy.c #256 crash case) and with a valid buffer