During my interop testing with stuntman (stun server) and its client library code, I found a couple of bugs in the "stund" tool that builds with libnice. I have provided a patch to fix the issue.<br><br>Issue: If stund tool receives a STUN binding request with an unknown attribute, the code will get into an infinite spinning loop and not process any more incoming packets.<br>
<br>Here is what is happening.<br><br>The dgram_process() function receives a datagram with a STUN binding request.<br><br>If stun_agent_validate() returns UNKNOWN_REQUEST_ATTRIBUTE (which it will do for almost any message with an attribute.)<br>
<br>Then stun_age_build_unknown_attributes_error() is called to build the message. Then a "goto send_buf".<br><br>Then send_safe is called. But the iov.iov_len hasn't been properly set. It's initial value is 65552, which is longer than the length of a valid UDP datagram. So now sendmsg() is called inside safe_send() and returns -1 (errno==EMSGSIZE). And then safe_send gets into an infinite loop and never returns.<br>
<br>The simple fix is to move the stun_agent_finish_message() call to be below the send_buf label instead of directly above it. Diff below.<br><br>Two other issues.<br><br>1) stun/stunmessage.h defines STUN_MAX_MESSAGE_SIZE to be 65552. For UDP, this is impossible, because the maximum value for a 16-bit length field in the UDP header is 65535. Subtract off the 8 byte UDP header, and that leaves a theoretical maximum for a datagram payload buffer to be 65527 (or 65507 if the IP header is factored in). And realistically, STUN messages don't get anywhere near that size.<br>
<br>2) Also, safe_send does not look safe to me. Since the socket isn't non-blocking or have any other special attributes, it doesn't make sense to keep repeatedly trying to call sendmsg() until it succeeds. (At least not without evaluating the errno). So in the diff below, I just removed it in favor of a direct call to sendmsg.<br>
<br>I found this issue with the the stuntman client library code, when it sends a binding request, it always sends a CHANGE-REQUEST attribute even if both the ip and port flags are both false. The reason I have to do this is because of a bug in JSTUN. The JSTUN server code has a bug that will reject any message that doesn't have a CHANGE-REQUEST attribute. But the stund tool with libnice will reject any binding request with any attribute in it (because the library call to tell the agent the acceptable attributes aren't there.)<br>
<br>Here's the diff for stund.c<br><br>201,213d200<br>< /** Sends a message through a socket */<br>< ssize_t send_safe (int fd, const struct msghdr *msg)<br>< {<br>< ssize_t len;<br>< <br>< do<br>
< len = sendmsg (fd, msg, 0);<br>< while ((len == -1) && (recv_err (fd) == 0));<br>< <br>< return len;<br>< }<br>< <br>< <br>282d268<br>< iov.iov_len = stun_agent_finish_message (agent, &response, NULL, 0);<br>
283a270<br>> iov.iov_len = stun_agent_finish_message (agent, &response, NULL, 0);<br>285,286c272,273<br>< len = send_safe (sock, &mh);<br>< return (len < iov.iov_len) ? -1 : 0;<br>---<br>> len = sendmsg (sock, &mh, 0);<br>
> return len;<br><br><br>John Selbie<br><br>