Bug 500 - Use of uninitialized variables as a result of create_udp_sock() not setting 'inuse' and 'noproto'
Use of uninitialized variables as a result of create_udp_sock() not setting '...
Status: RESOLVED FIXED
Product: unbound
Classification: Unclassified
Component: server
unspecified
All All
: P5 normal
Assigned To: unbound team
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-15 18:08 CEST by Jake Montgomery
Modified: 2013-05-16 10:15 CEST (History)
1 user (show)

See Also:


Attachments
Proposed fix for the unbound-1.4.20 version of listen_dnsport.c (658 bytes, patch)
2013-05-15 18:08 CEST, Jake Montgomery
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jake Montgomery 2013-05-15 18:08:37 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,
    Jake Montgomery
    Software Engineer
    Dyn Inc. - http://www.dyn.com/
Comment 1 Wouter Wijngaards 2013-05-16 10:15:39 CEST
Hi Jake,

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