search

trusteddomainproject / OpenDMARC

This is the Trusted Domain Project's impementation of the DMARC protocol libary and mail filter, called OpenDMARC. A "milter" connects to unix-based mailers (originally, sendmail, but now many) and provides a standard filtering API.
Other
102 stars 55 forks source link

dmarc_dns_get_record() calls res_ninit() without zeroing resp first #245

Open Bill-Sommerfeld opened 1 year ago

Bill-Sommerfeld commented 1 year ago

The documentation of res_ninit() is inconsistent across different operating systems but generally the caller must zero some or all of the argument structure before calling res_ninit()

This is done consistently in opendmarc_spf_dns.c (memset before res_ninit) but not in opendmarc_dns.c:

https://github.com/trusteddomainproject/OpenDMARC/blob/9cebf724d601452d1a671ed5331551dbc18df83a/libopendmarc/opendmarc_dns.c#L205-L207

I got a burst of crashes a few hours after enabling opendmarc:

libc.so.1`_free_unlocked+0x16()
libresolv.so.2`res_ndestroy+0x27(fffffc7fee919950)
libresolv.so.2`__res_vinit+0x45(fffffc7fee919950, 0)
libresolv.so.2`res_ninit+0x10(fffffc7fee919950)
libopendmarc.so.2.0.3`dmarc_dns_get_record+0x159(ffffffffffffffff, ffffffffffffffff, fffffc7fee91bbd0)
0xffffffffffffffff()

evidently due to non-zero stack garbage in the memory used for resp. Fix is straightforward:

--- libopendmarc/opendmarc_dns.c.~1~    Tue Sep  5 09:42:40 2023
+++ libopendmarc/opendmarc_dns.c    Tue Sep  5 09:42:57 2023
@@ -203,6 +203,7 @@
        ++bp;

 #ifdef HAVE_RES_NINIT   
+   memset(&resp, '\0', sizeof resp);
    res_ninit(&resp);
 #ifdef RES_USE_DNSSEC
    resp.options |= RES_USE_DNSSEC;
futatuki commented 5 months ago

I could confirm that your patch fixes the crash when the milter was called on the end of the message every times on FreeBSD 14.1-RELEASE. Thank you.

(On FreeBSD 14.0-RELEASE, HAVE_RES_NINIT was not defined because of the issue #257)