From 51ae1c8770369d8af3ea1fb0a24035d680491fc0 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Fri, 28 Aug 2020 18:46:36 -0700 Subject: [PATCH] Fix socket errors being clobbered by closesocket() --- miss/relay.cpp | 16 +++++++++------ mist/stun.cpp | 54 +++++++++++++++++++++++++++++--------------------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/miss/relay.cpp b/miss/relay.cpp index 1159ddb..37e32b1 100644 --- a/miss/relay.cpp +++ b/miss/relay.cpp @@ -68,11 +68,13 @@ int StartUdpRelay(unsigned short Port) SOCKADDR_IN addr; HANDLE thread; PUDP_TUPLE tuple; + int error; sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (sock == INVALID_SOCKET) { - printf("socket() failed: %d\n", WSAGetLastError()); - return WSAGetLastError(); + error = WSAGetLastError(); + printf("socket() failed: %d\n", error); + return error; } // Bind to the alternate port @@ -80,9 +82,10 @@ int StartUdpRelay(unsigned short Port) addr.sin_family = AF_INET; addr.sin_port = htons(Port + RELAY_PORT_OFFSET); if (bind(sock, (PSOCKADDR)&addr, sizeof(addr)) == SOCKET_ERROR) { - printf("bind() failed: %d\n", WSAGetLastError()); + error = WSAGetLastError(); + printf("bind() failed: %d\n", error); closesocket(sock); - return WSAGetLastError(); + return error; } tuple = (PUDP_TUPLE)malloc(sizeof(*tuple)); @@ -95,9 +98,10 @@ int StartUdpRelay(unsigned short Port) thread = CreateThread(NULL, 0, UdpRelayThreadProc, tuple, 0, NULL); if (thread == NULL) { - printf("CreateThread() failed: %d\n", GetLastError()); + error = GetLastError(); + printf("CreateThread() failed: %d\n", error); closesocket(sock); - return GetLastError(); + return error; } CloseHandle(thread); diff --git a/mist/stun.cpp b/mist/stun.cpp index 8c80af8..e1dc998 100644 --- a/mist/stun.cpp +++ b/mist/stun.cpp @@ -45,15 +45,20 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) int i; int bytesRead; int err; + bool ret; PSTUN_ATTRIBUTE_HEADER attribute; PSTUN_MAPPED_IPV4_ADDRESS_ATTRIBUTE ipv4Attrib; struct addrinfo* result; struct addrinfo hints; + struct sockaddr_in bindAddr; union { STUN_MESSAGE hdr; char buf[1024]; } resp; + sock = INVALID_SOCKET; + ret = false; + memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_INET; hints.ai_flags = AI_ADDRCONFIG; @@ -68,18 +73,15 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) sock = socket(hints.ai_family, hints.ai_socktype, hints.ai_protocol); if (sock == INVALID_SOCKET) { fprintf(LOG_OUT, "socket() failed: %d\n", WSAGetLastError()); - freeaddrinfo(result); - return false; + goto Exit; } - struct sockaddr_in bindAddr = {}; + bindAddr = {}; bindAddr.sin_family = hints.ai_family; bindAddr.sin_port = htons(localPort); if (bind(sock, (struct sockaddr*)&bindAddr, sizeof(bindAddr)) == SOCKET_ERROR) { fprintf(LOG_OUT, "bind() failed: %d\n", WSAGetLastError()); - closesocket(sock); - freeaddrinfo(result); - return false; + goto Exit; } reqMsg.messageType = htons(STUN_MESSAGE_BINDING_REQUEST); @@ -113,9 +115,7 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) } else if (selectRes == SOCKET_ERROR) { fprintf(LOG_OUT, "select() failed: %d\n", WSAGetLastError()); - closesocket(sock); - freeaddrinfo(result); - return false; + goto Exit; } // Error handling is below @@ -123,32 +123,29 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) break; } - freeaddrinfo(result); - closesocket(sock); - if (bytesRead == 0) { fprintf(LOG_OUT, "No response from STUN server\n"); - return false; + goto Exit; } else if (bytesRead == SOCKET_ERROR) { fprintf(LOG_OUT, "Failed to read STUN binding response: %d\n", WSAGetLastError()); - return false; + goto Exit; } else if (bytesRead < sizeof(resp.hdr)) { fprintf(LOG_OUT, "STUN message truncated: %d\n", bytesRead); - return false; + goto Exit; } else if (htonl(resp.hdr.magicCookie) != STUN_MESSAGE_COOKIE) { fprintf(LOG_OUT, "Bad STUN cookie value: %x\n", htonl(resp.hdr.magicCookie)); - return false; + goto Exit; } else if (memcmp(reqMsg.transactionId, resp.hdr.transactionId, sizeof(reqMsg.transactionId))) { fprintf(LOG_OUT, "STUN transaction ID mismatch\n"); - return false; + goto Exit; } else if (htons(resp.hdr.messageType) != STUN_MESSAGE_BINDING_SUCCESS) { fprintf(LOG_OUT, "STUN message type mismatch: %x\n", htons(resp.hdr.messageType)); - return false; + goto Exit; } attribute = (PSTUN_ATTRIBUTE_HEADER)(&resp.hdr + 1); @@ -156,7 +153,7 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) while (bytesRead > sizeof(*attribute)) { if (bytesRead < sizeof(*attribute) + htons(attribute->length)) { fprintf(LOG_OUT, "STUN attribute out of bounds: %d\n", htons(attribute->length)); - return false; + goto Exit; } // Mask off the comprehension bit else if ((htons(attribute->type) & 0x7FFF) != STUN_ATTRIBUTE_XOR_MAPPED_ADDRESS) { @@ -169,11 +166,11 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) ipv4Attrib = (PSTUN_MAPPED_IPV4_ADDRESS_ATTRIBUTE)attribute; if (htons(ipv4Attrib->hdr.length) != 8) { fprintf(LOG_OUT, "STUN address length mismatch: %d\n", htons(ipv4Attrib->hdr.length)); - return false; + goto Exit; } else if (ipv4Attrib->addressFamily != 1) { fprintf(LOG_OUT, "STUN address family mismatch: %x\n", ipv4Attrib->addressFamily); - return false; + goto Exit; } *wanAddr = {}; @@ -183,9 +180,20 @@ bool getExternalAddressPortIP4(unsigned short localPort, PSOCKADDR_IN wanAddr) wanAddr->sin_port = ipv4Attrib->port ^ (short)resp.hdr.magicCookie; wanAddr->sin_addr.S_un.S_addr = ipv4Attrib->address ^ resp.hdr.magicCookie; - return true; + ret = true; + goto Exit; } fprintf(LOG_OUT, "No XOR mapped address found in STUN response!\n"); - return false; + +Exit: + if (sock != INVALID_SOCKET) { + closesocket(sock); + } + + if (result != NULL) { + freeaddrinfo(result); + } + + return ret; } \ No newline at end of file