Bugzilla – Bug 500
Use of uninitialized variables as a result of create_udp_sock() not setting 'inuse' and 'noproto'
Last modified: 2013-05-16 10:15:39 CEST
Created attachment 223 [details]
Proposed fix for the unbound-1.4.20 version of listen_dnsport.c
This issue, or a variation of it, appears in unbound-1.4.20, and goes back to, at least, unbound-1.4.12, which is the earliest version checked. All code references here are for unbound-1.4.20.
The function create_udp_sock()in listen_dnsport.c takes two pointers, named 'inuse' and 'noproto'. Looking at the use cases for create_udp_sock(), it looks like these variables are only used when create_udp_sock()returns -1. In addition, these variables are not initialized before being passed to create_udp_sock(). As a result, it should be required that they be set by create_udp_sock() whenever it returns -1. This is already done in most, but not all, code paths that return -1.
Attached is a proposed patch for listen_dnsport.c in unbound-1.4.20. This patch fixes the two cases where neither are set, around lines 331, and 344. It also adds an assignment to 'inuse' at line 349, since the assignment of 'inuse' just below it only applies if USE_WINSOCK is not defined and EADDRINUSE is defined.
Instead of this patch, it might be cleaner to take the approach used for 'noproto' in the function create_tcp_accept_sock(), and set both 'inuse' and 'noproto' to 0 st the start of create_udp_sock(), and factor out the redundant assignments.
Hope this helps,
Dyn Inc. - http://www.dyn.com/
Thanks for this patch. I applied it, and not the refactored code, because this code binds udp sockets for use and this happens for every message. A couple less pointer manipulations for the normal-working case is a good thing for speed.
Best regards, Wouter