FIXED: double free or corruption #21

Closed
srinivasahebbar opened this Issue Sep 23, 2015 · 10 comments

Comments

Projects
None yet
2 participants
@srinivasahebbar

Dear Sir,

Deadwood version 3.2.8 crashes when no default present in the system.
3.2.7 also exhibits the same crash problem.

I have tested this both on x86(linux) as well as mips (linux) build and both version
crashes similarly when default route on the system is not present.

I have attached strace output as well as config file. The problem is very 100% consistent.

ERROR:

*** glibc detected *** ./Deadwood: double free or corruption (out): 0x09c16b30 ***

Thanks,
Srinivasa

CONFIG_FILE

ipv4_bind_addresses="0.0.0.0"
chroot_dir="/tmp/maradns"
maxprocs=512
handle_overload=1

ttl_age=1
max_ar_chain=1
max_inflights=8
reject_aaaa=1
reject_mx=0
filter_rfc1918=0

maradns_uid=1000
maradns_gid=1000

recursive_acl="0.0.0.0/0"

maximum_cache_elements=512
timeout_seconds=2
verbose_level=3000

root_servers = {}
root_servers["."] = "198.41.0.4,192.228.79.201,192.33.4.12,199.7.91.13,192.203.230.10,192.5.5.241"
root_servers["."] += "192.112.36.4,128.63.2.53,192.36.148.17,192.58.128.30,193.0.14.129,199.7.83.42,202.12.27.33"

STRACE_OUTPUT on X86
write(1, "Got DNS query for \014toptoptopt"..., 53Got DNS query for \014toptoptoptop\002tt\000\000\001
) = 53
write(1, "Looking in cache for query \014t"..., 62Looking in cache for query \014toptoptoptop\002tt\000\000\001
) = 62
write(1, "Nothing found for \014toptoptopt"..., 53Nothing found for \014toptoptoptop\002tt\000\000\001
) = 53
write(1, "Making connection to IP 192.36.1"..., 39Making connection to IP 192.36.148.17
) = 39
socket(PF_INET, SOCK_DGRAM, IPPROTO_IP) = 4
fcntl64(4, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
bind(4, {sa_family=AF_INET, sin_port=htons(15035), sin_addr=inet_addr("0.0.0.0")}, 16) = 0
connect(4, {sa_family=AF_INET, sin_port=htons(53), sin_addr=inet_addr("192.36.148.17")}, 16) = -1 ENETUNREACH (Network is unreachable)
close(4) = 0
open("/dev/tty", O_RDWR|O_NOCTTY|O_NONBLOCK) = -1 ENOENT (No such file or directory)
writev(2, [{"*** glibc detected *** ", 23}, {"./Deadwood", 10}, {": ", 2}, {"double free or corruption (out)", 31}, {": 0x", 4}, {"0930bb30", 8}, {" _\n", 5}], 7_ glibc detected *** ./Deadwood: double free or corruption (out): 0x0930bb30 ***
) = 83
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb76f6000
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

@srinivasahebbar

This comment has been minimized.

Show comment
Hide comment
@srinivasahebbar

srinivasahebbar Sep 23, 2015

The version 3.2.5 works perfect.
3.2.6: Not tested
3.2.7: crashed when default route NOT present
3.2.8: crashed when default route NOT present

The version 3.2.5 works perfect.
3.2.6: Not tested
3.2.7: crashed when default route NOT present
3.2.8: crashed when default route NOT present

@samboy

This comment has been minimized.

Show comment
Hide comment
@samboy

samboy Sep 23, 2015

Owner

Do you know how to compile Deadwood with “-g” and use gdb to supply a stack trace?

Bascially:

cd deadwood-github/src
export FLAGS=-g
make clean ; make
gdb ./Deadwood

Then type in “r” at the gdb prompt to start Deadwood. This will make a stack trace. Strace is useless for these kinds of bugs.

Owner

samboy commented Sep 23, 2015

Do you know how to compile Deadwood with “-g” and use gdb to supply a stack trace?

Bascially:

cd deadwood-github/src
export FLAGS=-g
make clean ; make
gdb ./Deadwood

Then type in “r” at the gdb prompt to start Deadwood. This will make a stack trace. Strace is useless for these kinds of bugs.

@srinivasahebbar

This comment has been minimized.

Show comment
Hide comment
@srinivasahebbar

srinivasahebbar Sep 23, 2015

GDB OUTPUT

Deadwood version 3.2.08
Deadwood: A DNS UDP non-recursive cache (IPv4-only)
Verbose_level set to 3000
We bound to 1 addresses
add_constant is set to 0x40c01d12
Got DNS query for \005gmail\003com\000\000\001
Looking in cache for query \005gmail\003com\000\000\001
Nothing found for \005gmail\003com\000\000\001
Making connection to IP 199.7.83.42
*** glibc detected *** /tmp/deadwood-3.2.08/src/Deadwood: double free or corruption (out): 0x08069b30 ***

Program received signal SIGABRT, Aborted.
0xb7fdd428 in __kernel_vsyscall ()
(gdb) bt
#0 0xb7fdd428 in __kernel_vsyscall ()
#1 0xb7e3be0f in raise () from /lib/i386-linux-gnu/libc.so.6
#2 0xb7e3f455 in abort () from /lib/i386-linux-gnu/libc.so.6
#3 0xb7e7843a in ?? () from /lib/i386-linux-gnu/libc.so.6
#4 0xb7e82f82 in ?? () from /lib/i386-linux-gnu/libc.so.6
#5 0x0804ca1b in reset_rem (a=0) at DwSocket.c:139
#6 0x0804f630 in forward_local_udp_packet (sock=7, local_id=28973, from_ip=0xbffff5dc, from_port=35375, a=0xbffff3c2 "\212\212", len=27, tcp_num=-1, orig_query=0x806a188)
at DwUdpSocket.c:414
#7 0x0804ff42 in try_forward_local_udp_packet (sock=7, local_id=28973, from_ip=0xbffff5dc, from_port=35375, packet=0xbffff3c2 "\212\212", len=27, client=0xbffff5cc, query=0x8069310,
tcp_num=-1, orig_query=0x806a188) at DwUdpSocket.c:636
#8 0x080504a7 in get_local_udp_packet (sock=7) at DwUdpSocket.c:754
#9 0x0804e070 in process_results (a=1, rx_fd=0xbffff65c) at DwSocket.c:822
#10 0x0804e97f in bigloop () at DwSocket.c:996
#11 0x08048f8e in dw_udp_main (argc=3, argv=0xbffff7f4) at DwMain.c:115
#12 0x080490d2 in main (argc=3, argv=0xbffff7f4) at DwMain.c:157

Line 139 of file DwSocket.c is : free(rem[a].local); in function void reset_rem(int_fast32_t a)

/* Reset the values for a remote connection */
void reset_rem(int_fast32_t a) {
if(rem[a].socket != INVALID_SOCKET) {
closesocket(rem[a].socket);
rem[a].socket = INVALID_SOCKET;
}
rem[a].die = 0;
rem[a].remote_id = 0;
rem[a].retries = num_retries;
if(rem[a].ns != 0) {
dw_destroy(rem[a].ns);
rem[a].ns = 0;
}
rem[a].is_upstream = 0;
if(rem[a].query != 0) {
zap_inflight(rem[a].query);
dw_destroy(rem[a].query);
rem[a].query = 0;
}
rem[a].recurse_depth = 0;
rem[a].current_ns = -1;
rem[a].child_id = -1;
if(rem[a].glueless != 0) {
dw_destroy(rem[a].glueless);
rem[a].glueless = 0;
}
if(rem[a].local != 0) {
int qq;
for(qq = 0; qq < rem[a].num_locals; qq++) {
if(rem[a].local[qq] != 0) {
dw_destroy(rem[a].local[qq]->orig_query);
dw_destroy(rem[a].local[qq]->action);
free(rem[a].local[qq]);
rem[a].local[qq] = 0; }
}
free(rem[a].local); <======== LINE 139
}
rem[a].local = 0;
rem[a].num_locals = 0;
}

GDB OUTPUT

Deadwood version 3.2.08
Deadwood: A DNS UDP non-recursive cache (IPv4-only)
Verbose_level set to 3000
We bound to 1 addresses
add_constant is set to 0x40c01d12
Got DNS query for \005gmail\003com\000\000\001
Looking in cache for query \005gmail\003com\000\000\001
Nothing found for \005gmail\003com\000\000\001
Making connection to IP 199.7.83.42
*** glibc detected *** /tmp/deadwood-3.2.08/src/Deadwood: double free or corruption (out): 0x08069b30 ***

Program received signal SIGABRT, Aborted.
0xb7fdd428 in __kernel_vsyscall ()
(gdb) bt
#0 0xb7fdd428 in __kernel_vsyscall ()
#1 0xb7e3be0f in raise () from /lib/i386-linux-gnu/libc.so.6
#2 0xb7e3f455 in abort () from /lib/i386-linux-gnu/libc.so.6
#3 0xb7e7843a in ?? () from /lib/i386-linux-gnu/libc.so.6
#4 0xb7e82f82 in ?? () from /lib/i386-linux-gnu/libc.so.6
#5 0x0804ca1b in reset_rem (a=0) at DwSocket.c:139
#6 0x0804f630 in forward_local_udp_packet (sock=7, local_id=28973, from_ip=0xbffff5dc, from_port=35375, a=0xbffff3c2 "\212\212", len=27, tcp_num=-1, orig_query=0x806a188)
at DwUdpSocket.c:414
#7 0x0804ff42 in try_forward_local_udp_packet (sock=7, local_id=28973, from_ip=0xbffff5dc, from_port=35375, packet=0xbffff3c2 "\212\212", len=27, client=0xbffff5cc, query=0x8069310,
tcp_num=-1, orig_query=0x806a188) at DwUdpSocket.c:636
#8 0x080504a7 in get_local_udp_packet (sock=7) at DwUdpSocket.c:754
#9 0x0804e070 in process_results (a=1, rx_fd=0xbffff65c) at DwSocket.c:822
#10 0x0804e97f in bigloop () at DwSocket.c:996
#11 0x08048f8e in dw_udp_main (argc=3, argv=0xbffff7f4) at DwMain.c:115
#12 0x080490d2 in main (argc=3, argv=0xbffff7f4) at DwMain.c:157

Line 139 of file DwSocket.c is : free(rem[a].local); in function void reset_rem(int_fast32_t a)

/* Reset the values for a remote connection */
void reset_rem(int_fast32_t a) {
if(rem[a].socket != INVALID_SOCKET) {
closesocket(rem[a].socket);
rem[a].socket = INVALID_SOCKET;
}
rem[a].die = 0;
rem[a].remote_id = 0;
rem[a].retries = num_retries;
if(rem[a].ns != 0) {
dw_destroy(rem[a].ns);
rem[a].ns = 0;
}
rem[a].is_upstream = 0;
if(rem[a].query != 0) {
zap_inflight(rem[a].query);
dw_destroy(rem[a].query);
rem[a].query = 0;
}
rem[a].recurse_depth = 0;
rem[a].current_ns = -1;
rem[a].child_id = -1;
if(rem[a].glueless != 0) {
dw_destroy(rem[a].glueless);
rem[a].glueless = 0;
}
if(rem[a].local != 0) {
int qq;
for(qq = 0; qq < rem[a].num_locals; qq++) {
if(rem[a].local[qq] != 0) {
dw_destroy(rem[a].local[qq]->orig_query);
dw_destroy(rem[a].local[qq]->action);
free(rem[a].local[qq]);
rem[a].local[qq] = 0; }
}
free(rem[a].local); <======== LINE 139
}
rem[a].local = 0;
rem[a].num_locals = 0;
}

@samboy

This comment has been minimized.

Show comment
Hide comment
@samboy

samboy Sep 23, 2015

Owner

Thanks: That helps a lot. I hope to be able to push out a fix by next week.

Owner

samboy commented Sep 23, 2015

Thanks: That helps a lot. I hope to be able to push out a fix by next week.

samboy added a commit that referenced this issue Sep 25, 2015

#21
If a connection to an upstream IP fails, the output of malloc()
or 0 could be written to an out of bounds memory location.
@samboy

This comment has been minimized.

Show comment
Hide comment
@samboy

samboy Sep 25, 2015

Owner

OK, fixed. Here’s what happens:

In forward_local_udp_packet(), after resetting the connection information, we start connecting using make_new_udp_connect()

make_new_udp_connect() calls make_remote_connection() to make the actual connection, which fails because there isn’t a default route.

At this point rem[b].local is allocated, but the number of local connections is not correctly set to one; rem[b].num_locals is still 0

Then we have this code:

        rem[b].local[rem[b].num_locals - 1] = dw_malloc(sizeof(local_T));
        if(rem[b].local[rem[b].num_locals - 1] != 0) {
                rem[b].local[rem[b].num_locals - 1]->orig_query = 0;
                rem[b].local[rem[b].num_locals - 1]->action = 0;
        }

Which writes to rem[b].local[-1] with the output of malloc().

To fix this, I have added a sanity test just above that code:

        if(rem[b].num_locals == 0) {
                reset_rem(b);
                return -1;
        }

Closing. Let me know if this doesn’t fix your issue.

I will release Deadwood 3.2.08 and MaraDNS 2.0.13 with this bug patched this weekend.

No, this is not something from the 2001-2002 codebase; this is from the 2009 codebase when I added code to merge multiple inflight connections, to protect against attacks like https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-4392 (Yeah, spoofing is much more dangerous than being able to possibly remotely crash Deadwood, so I made the right call)

Owner

samboy commented Sep 25, 2015

OK, fixed. Here’s what happens:

In forward_local_udp_packet(), after resetting the connection information, we start connecting using make_new_udp_connect()

make_new_udp_connect() calls make_remote_connection() to make the actual connection, which fails because there isn’t a default route.

At this point rem[b].local is allocated, but the number of local connections is not correctly set to one; rem[b].num_locals is still 0

Then we have this code:

        rem[b].local[rem[b].num_locals - 1] = dw_malloc(sizeof(local_T));
        if(rem[b].local[rem[b].num_locals - 1] != 0) {
                rem[b].local[rem[b].num_locals - 1]->orig_query = 0;
                rem[b].local[rem[b].num_locals - 1]->action = 0;
        }

Which writes to rem[b].local[-1] with the output of malloc().

To fix this, I have added a sanity test just above that code:

        if(rem[b].num_locals == 0) {
                reset_rem(b);
                return -1;
        }

Closing. Let me know if this doesn’t fix your issue.

I will release Deadwood 3.2.08 and MaraDNS 2.0.13 with this bug patched this weekend.

No, this is not something from the 2001-2002 codebase; this is from the 2009 codebase when I added code to merge multiple inflight connections, to protect against attacks like https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-4392 (Yeah, spoofing is much more dangerous than being able to possibly remotely crash Deadwood, so I made the right call)

@samboy samboy closed this Sep 25, 2015

@samboy

This comment has been minimized.

Show comment
Hide comment
Owner

samboy commented Sep 25, 2015

@samboy

This comment has been minimized.

Show comment
Hide comment
@samboy

samboy Sep 26, 2015

Owner

Deadwood 3.2.09 released with this bug fix:

http://maradns.samiam.org/deadwood/stable/

Owner

samboy commented Sep 26, 2015

Deadwood 3.2.09 released with this bug fix:

http://maradns.samiam.org/deadwood/stable/

@srinivasahebbar

This comment has been minimized.

Show comment
Hide comment
@srinivasahebbar

srinivasahebbar Sep 26, 2015

Release 3.2.9 deadwood is not crashing when default router doesn't exist.

Thank you.

Release 3.2.9 deadwood is not crashing when default router doesn't exist.

Thank you.

@samboy

This comment has been minimized.

Show comment
Hide comment
@samboy

samboy Sep 26, 2015

Owner

Thanks for confirming that I fixed the bug. I have just uploaded MaraDNS 2.0.13, which has this as well as #20 + #19 fixed:

http://maradns.samiam.org/download/2.0/2.0.13/

Owner

samboy commented Sep 26, 2015

Thanks for confirming that I fixed the bug. I have just uploaded MaraDNS 2.0.13, which has this as well as #20 + #19 fixed:

http://maradns.samiam.org/download/2.0/2.0.13/

@samboy

This comment has been minimized.

Show comment
Hide comment
@samboy

samboy Sep 27, 2015

Owner

I just verified that the 2.3 branch of Deadwood doesn’t have inflight merging, so it doesn’t have this bug.

In flight merging (and this bug) was added on August 31, 2009, in Deadwood 2.4.07 http://maradns.blogspot.com/2009/08/deadwood-2407-released.html

Owner

samboy commented Sep 27, 2015

I just verified that the 2.3 branch of Deadwood doesn’t have inflight merging, so it doesn’t have this bug.

In flight merging (and this bug) was added on August 31, 2009, in Deadwood 2.4.07 http://maradns.blogspot.com/2009/08/deadwood-2407-released.html

@samboy samboy changed the title from double free or corruption to FIXED: double free or corruption May 6, 2016

Repository owner locked and limited conversation to collaborators May 21, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.