OpenDMARC icon indicating copy to clipboard operation
OpenDMARC copied to clipboard

dmarc_dns_get_record() calls res_ninit() without zeroing resp first

Open Bill-Sommerfeld opened this issue 2 years ago • 1 comments

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;

Bill-Sommerfeld avatar Sep 05 '23 19:09 Bill-Sommerfeld

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)

futatuki avatar Jun 15 '24 21:06 futatuki