From: Willem Toorop Date: Mon, 15 Jul 2019 10:38:48 +0000 (+0200) Subject: Bugfix #279 Bad return value for ldns_udp_connect and ldns_tcp_connect X-Git-Tag: release-1.7.1-rc1~9^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c0d066a5246b3a4cb153e169226b790eab7de361;p=thirdparty%2Fldns.git Bugfix #279 Bad return value for ldns_udp_connect and ldns_tcp_connect ldns_udp_connect, ldns_tcp_connect, ldns_udp_bgsend and ldns_tcp_bgsend return a socket and otherwise 0 on failure. This is wrong because 0 can be a valid socket. To not mess with the existing API, ldns_udp_connect2, ldns_tcp_connect2, ldns_udp_bgsend2 and ldns_tcp_bgsend2 are introduced that do the same but return -1 on failure. Internal usage of the old functions replaced with the new ones. --- diff --git a/configure.ac b/configure.ac index 845cd999..48e34357 100644 --- a/configure.ac +++ b/configure.ac @@ -1046,10 +1046,10 @@ size_t strlcpy(char *dst, const char *src, size_t siz); #ifdef USE_WINSOCK #define SOCK_INVALID INVALID_SOCKET -#define close_socket(_s) do { if (_s > SOCK_INVALID) {closesocket(_s); _s = SOCK_INVALID;} } while(0) +#define close_socket(_s) do { if (_s != SOCK_INVALID) {closesocket(_s); _s = -1;} } while(0) #else #define SOCK_INVALID -1 -#define close_socket(_s) do { if (_s > SOCK_INVALID) {close(_s); _s = SOCK_INVALID;} } while(0) +#define close_socket(_s) do { if (_s != SOCK_INVALID) {close(_s); _s = -1;} } while(0) #endif #ifdef __cplusplus diff --git a/ldns/net.h.in b/ldns/net.h.in index 9faf3a7d..25912582 100644 --- a/ldns/net.h.in +++ b/ldns/net.h.in @@ -48,7 +48,20 @@ ldns_status ldns_udp_send(uint8_t **result, ldns_buffer *qbin, const struct sock * \param[in] to the ip addr to send to * \param[in] tolen length of the ip addr * \param[in] timeout *unused*, was the timeout value for the network - * \return the socket used + * \return the socket used or -1 on failure + */ +int ldns_udp_bgsend2(ldns_buffer *qbin, const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout); + +/** + * Send an udp query and don't wait for an answer but return + * the socket + * This function has the flaw that it returns 0 on failure, but 0 could be a + * valid socket. Please use ldns_udp_bgsend2 instead of this function. + * \param[in] qbin the ldns_buffer to be send + * \param[in] to the ip addr to send to + * \param[in] tolen length of the ip addr + * \param[in] timeout *unused*, was the timeout value for the network + * \return the socket used or 0 on failure */ int ldns_udp_bgsend(ldns_buffer *qbin, const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout); @@ -59,7 +72,20 @@ int ldns_udp_bgsend(ldns_buffer *qbin, const struct sockaddr_storage *to, sockle * \param[in] to the ip addr to send to * \param[in] tolen length of the ip addr * \param[in] timeout the timeout value for the connect attempt - * \return the socket used + * \return the socket used or -1 on failure + */ +int ldns_tcp_bgsend2(ldns_buffer *qbin, const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout); + +/** + * Send an tcp query and don't wait for an answer but return + * the socket + * This function has the flaw that it returns 0 on failure, but 0 could be a + * valid socket. Please use ldns_tcp_bgsend2 instead of this function. + * \param[in] qbin the ldns_buffer to be send + * \param[in] to the ip addr to send to + * \param[in] tolen length of the ip addr + * \param[in] timeout the timeout value for the connect attempt + * \return the socket used or 0 on failure */ int ldns_tcp_bgsend(ldns_buffer *qbin, const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout); @@ -104,7 +130,18 @@ ldns_status ldns_send_buffer(ldns_pkt **pkt, ldns_resolver *r, ldns_buffer *qb, * \param[in] to ip and family * \param[in] tolen length of to * \param[in] timeout timeout for the connect attempt - * \return a socket descriptor + * \return a socket descriptor or -1 on failure + */ +int ldns_tcp_connect2(const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout); + +/** + * Create a tcp socket to the specified address + * This function has the flaw that it returns 0 on failure, but 0 could be a + * valid socket. Please use ldns_tcp_connect2 instead of this function. + * \param[in] to ip and family + * \param[in] tolen length of to + * \param[in] timeout timeout for the connect attempt + * \return a socket descriptor or 0 on failure */ int ldns_tcp_connect(const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout); @@ -112,7 +149,17 @@ int ldns_tcp_connect(const struct sockaddr_storage *to, socklen_t tolen, struct * Create a udp socket to the specified address * \param[in] to ip and family * \param[in] timeout *unused*, was timeout for the socket - * \return a socket descriptor + * \return a socket descriptor or -1 on failure + */ +int ldns_udp_connect2(const struct sockaddr_storage *to, struct timeval timeout); + +/** + * Create a udp socket to the specified address + * This function has the flaw that it returns 0 on failure, but 0 could be a + * valid socket. Please use ldns_udp_connect2 instead of this function. + * \param[in] to ip and family + * \param[in] timeout *unused*, was timeout for the socket + * \return a socket descriptor or 0 on failure */ int ldns_udp_connect(const struct sockaddr_storage *to, struct timeval timeout); diff --git a/net.c b/net.c index a296cc8b..13bfc972 100644 --- a/net.c +++ b/net.c @@ -198,12 +198,12 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen, #ifndef S_SPLINT_S if ((sockfd = socket((int)((struct sockaddr*)to)->sa_family, SOCK_STREAM, IPPROTO_TCP)) == SOCK_INVALID) { - return 0; + return -1; } #endif if (from && bind(sockfd, (const struct sockaddr*)from, fromlen) == SOCK_INVALID){ close_socket(sockfd); - return 0; + return -1; } /* perform nonblocking connect, to be able to wait with select() */ @@ -216,13 +216,13 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen, if(1) { #endif close_socket(sockfd); - return 0; + return -1; } #else /* USE_WINSOCK */ if(WSAGetLastError() != WSAEINPROGRESS && WSAGetLastError() != WSAEWOULDBLOCK) { close_socket(sockfd); - return 0; + return -1; } #endif /* error was only telling us that it would block */ @@ -235,7 +235,7 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen, if(!ldns_sock_wait(sockfd, timeout, 1)) { close_socket(sockfd); - return 0; + return -1; } /* check if there is a pending error for nonblocking connect */ @@ -256,7 +256,7 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen, close_socket(sockfd); /* error in errno for our user */ errno = error; - return 0; + return -1; } #else /* USE_WINSOCK */ if(error == WSAEINPROGRESS) @@ -266,7 +266,7 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen, else if(error != 0) { close_socket(sockfd); errno = error; - return 0; + return -1; } #endif /* USE_WINSOCK */ /* connected */ @@ -282,6 +282,14 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen, int ldns_tcp_connect(const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout) +{ + int s = ldns_tcp_connect_from(to, tolen, NULL, 0, timeout); + return s > 0 ? s : 0; +} + +int +ldns_tcp_connect2(const struct sockaddr_storage *to, socklen_t tolen, + struct timeval timeout) { return ldns_tcp_connect_from(to, tolen, NULL, 0, timeout); } @@ -296,13 +304,9 @@ ldns_tcp_bgsend_from(ldns_buffer *qbin, sockfd = ldns_tcp_connect_from(to, tolen, from, fromlen, timeout); - if (sockfd == 0) { - return 0; - } - - if (ldns_tcp_send_query(qbin, sockfd, to, tolen) == 0) { + if (sockfd >= 0 && ldns_tcp_send_query(qbin, sockfd, to, tolen) == 0) { close_socket(sockfd); - return 0; + return -1; } return sockfd; @@ -313,9 +317,17 @@ ldns_tcp_bgsend(ldns_buffer *qbin, const struct sockaddr_storage *to, socklen_t tolen, struct timeval timeout) { - return ldns_tcp_bgsend_from(qbin, to, tolen, NULL, 0, timeout); + int s = ldns_tcp_bgsend_from(qbin, to, tolen, NULL, 0, timeout); + return s > 0 ? s : 0; } +int +ldns_tcp_bgsend2(ldns_buffer *qbin, + const struct sockaddr_storage *to, socklen_t tolen, + struct timeval timeout) +{ + return ldns_tcp_bgsend_from(qbin, to, tolen, NULL, 0, timeout); +} /* keep in mind that in DNS tcp messages the first 2 bytes signal the * amount data to expect @@ -331,7 +343,7 @@ ldns_tcp_send_from(uint8_t **result, ldns_buffer *qbin, sockfd = ldns_tcp_bgsend_from(qbin, to, tolen, from, fromlen, timeout); - if (sockfd == 0) { + if (sockfd == -1) { return LDNS_STATUS_ERR; } @@ -369,13 +381,28 @@ ldns_udp_connect(const struct sockaddr_storage *to, struct timeval ATTR_UNUSED(t #ifndef S_SPLINT_S if ((sockfd = socket((int)((struct sockaddr*)to)->sa_family, SOCK_DGRAM, IPPROTO_UDP)) - == -1) { + == SOCK_INVALID) { return 0; } #endif return sockfd; } +int +ldns_udp_connect2(const struct sockaddr_storage *to, struct timeval ATTR_UNUSED(timeout)) +{ + int sockfd; + +#ifndef S_SPLINT_S + if ((sockfd = socket((int)((struct sockaddr*)to)->sa_family, SOCK_DGRAM, + IPPROTO_UDP)) + == SOCK_INVALID) { + return -1; + } +#endif + return sockfd; +} + static int ldns_udp_bgsend_from(ldns_buffer *qbin, const struct sockaddr_storage *to , socklen_t tolen, @@ -384,20 +411,20 @@ ldns_udp_bgsend_from(ldns_buffer *qbin, { int sockfd; - sockfd = ldns_udp_connect(to, timeout); + sockfd = ldns_udp_connect2(to, timeout); - if (sockfd == 0) { - return 0; + if (sockfd == -1) { + return -1; } if (from && bind(sockfd, (const struct sockaddr*)from, fromlen) == -1){ close_socket(sockfd); - return 0; + return -1; } if (ldns_udp_send_query(qbin, sockfd, to, tolen) == 0) { close_socket(sockfd); - return 0; + return -1; } return sockfd; } @@ -406,6 +433,15 @@ int ldns_udp_bgsend(ldns_buffer *qbin, const struct sockaddr_storage *to , socklen_t tolen, struct timeval timeout) +{ + s = ldns_udp_bgsend_from(qbin, to, tolen, NULL, 0, timeout); + return s > 0 ? s : 0; +} + +int +ldns_udp_bgsend2(ldns_buffer *qbin, + const struct sockaddr_storage *to , socklen_t tolen, + struct timeval timeout) { return ldns_udp_bgsend_from(qbin, to, tolen, NULL, 0, timeout); } @@ -421,7 +457,7 @@ ldns_udp_send_from(uint8_t **result, ldns_buffer *qbin, sockfd = ldns_udp_bgsend_from(qbin, to, tolen, from, fromlen, timeout); - if (sockfd == 0) { + if (sockfd == -1) { return LDNS_STATUS_SOCKET_ERROR; } diff --git a/resolver.c b/resolver.c index 5a1db901..a1a746e5 100644 --- a/resolver.c +++ b/resolver.c @@ -1533,14 +1533,14 @@ void ldns_axfr_abort(ldns_resolver *resolver) { /* Only abort if an actual AXFR is in progress */ - if (resolver->_socket != 0) + if (resolver->_socket != -1) { #ifndef USE_WINSOCK close(resolver->_socket); #else closesocket(resolver->_socket); #endif - resolver->_socket = 0; + resolver->_socket = -1; } }