#164 Buffer overflow in libslp Previous Responder handling

[Alan]

In libslp_network.c the handling of prlist across multiple functions (NetworkRqstRply, NetworkMcastRqstRply) checks available buffer space with "if ( prlistlen + addrstrlen + 1 < mtu )", but then uses strcpy() instead of memcpy() to put bytes into the buffer. The "+ 1" only accounts for the comma delimiter, and does not account for the unnecessary NULL terminator that strcpy() and strcat() will include.

Therefore we can end up overrunning the available buffer by the one additional byte, as PageHeap has caught libslp doing on the Windows platform in a recent customer investigation.

It appears there was already an effort to address this in NetworkMcastRqstRply, through the introduction of prlistsize and xrealloc(). I respectfully suggest that we don't need for the prlist to be reallocated above the mtu size, and that the "overwrite by one byte because of the unnecessary NULL terminator" is and has been the only real issue here. Regardless of which fix approach we settle on, the fix was never applied to NetworkRqstRply handing of prlist.

Kudos for already involving SLPContainsStringList in the prlist update as compared to earlier versions, as our intention is clearly to never put the same responder address into the list twice. A customer with misbehaving multicast forwarding recently exposed what happens when the exact same message is received multiple times by libslp in the older code, before SLPContainsStringList had been introduced to the prlist handling.