[ntp:security] [Bug 527] ntpd frequently crashes on Windows systems

Danny Mayer via the NTP Bugzilla bugzilla at ntp.isc.org
Thu May 24 04:43:44 PDT 2007


http://bugs.ntp.isc.org/527



----------------------------------------------------------------------------
Additional Comments From mayer at ntp.org (Danny Mayer)
Submitted on 2007-05-24 11:43

I got it.

First let me explain what the problem was. In the function
QueueSocketRecv()there are two lines (I'm skipping irrelevant lines):

int AddrLen;
if (SOCKET_ERROR == WSARecvFrom(buff->fd, &buff->wsabuff, 1, 
				&BytesReceived, &Flags, 
				(struct sockaddr *) &buff->recv_srcadr,
				(LPINT) &AddrLen, 
				(LPOVERLAPPED) lpo, NULL))

Now AddrLen is local to the function. However, the WSARecvFrom() lpFromlen
argument is an [in,out] parameter where you pass the length of the buffer that
is to hold the address of the sender of the packet and when the receive
completes it puts into that location the actual length of the received address.
Of course AddrLen is a variable LOCAL to the function and the resultant length
gets written to the address passed. However since WASRecvFrom()operates
asynchronously that assignment only happens when the packet is received and not
when it's set up. Of course that variable location no longer exists in memory
and as a result the completion clobbers whatever is at that address with data
that shouldn't go there. That, by the way, is the reason that it's getting an
error accessing 00000010 which is not coincidentally the length of the source
address returned - 16.

The reason that none of this can be caught by any of the tools now becomes
obvious. The function call is right but there is no way any tool is going to
know that the address length is going to be filled in later after it's exited
this function.

The fix I've implemented adds a length variable to the recvbuf structure and I
use that to pass to the WSARecvFrom() function.

In doing all of this I have done some additional cleanup work in the code which
will also handle additional potential issues and I've updated the
GetReceivedBuffers() code to not exit if it hasn't received a packet.

I'll be getting all of this into the repository shortly.

Heiko, I really do need you to test this code. Note that this is ntp-stable code
but can be applied to ntp-dev almost unchanged.

Danny

-- 
Danny Mayer <mayer at ntp.org>



------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


More information about the security mailing list