wahern / dns

dns.c: Single file non-blocking DNS C library without callbacks or external dependencies.
http://25thandclement.com/~william/projects/dns.c.html
MIT License
256 stars 64 forks source link

Segmentation fault in dns_res_frame_init when dns_so_init fails #12

Closed sjakub closed 8 years ago

sjakub commented 8 years ago

When dns_so_init in dns_res_open fails, it performs a cleanup. However, dns_resolver is not fully configured by that point yet. It calls dns_res_close(), which results in those calls: dns_res_reset(), dns_res_frame_init().

dns_res_frame_init() sets some options on the frame, based on the resconf configuration. But at that point resconf is not set, and the program crashes. There should be a check in dns_res_frame_init to make sure that R->resconf even valid.

The code that fails:

static void dns_res_frame_init(struct dns_resolver *R, struct dns_res_frame *frame) {
    memset(frame, '\0', sizeof *frame);

    if (!R->resconf->options.recurse)
        frame->qflags |= DNS_Q_RD;
    if (R->resconf->options.edns0)
        frame->qflags |= DNS_Q_EDNS0;
} /* dns_res_frame_init() */

I'm not sure why the first flag is SET when the option is NOT set, but if resconf is not set it probably doesn't really matter...

sjakub commented 8 years ago

And that problem was introduced in this change: 22bfa5c (by @wahern )

wahern commented 8 years ago

Thank you for the bug report. I haven't tagged a release yet because the recent refactor was kind of large and I still want to run more tests.

Regarding your comment, options.recurse tells the library to use recursion (i.e. query the TLD name servers and work its way down). DNS_Q_RD is a flag that requests setting the "recursion desired" header flag when building a query packet, which is needed when the resolver is configured as a a stub resolver. frame.qflags came about as a way to help centralize some query packet building code which was duplicated in different places in dns_res_exec.

sjakub commented 8 years ago

So setting "options.recurse" means that DNS_Q_RD (and consequently "recursion desired") will NOT be set? Feels counter-intuitive ;)