posixSocket::resolve may wreak havoc

[RichardSteele]

Invalid memory accesses are probable if posixSocket::resolve uses getaddrinfo_a and getaddrinfo_a doesn't finish within m_timeoutHandler's defined interval. The default timeout of 30 seconds makes the problem unlikely but a very low timeout or slow name resolution may reveal it.

The root cause is:

vmime/src/vmime/platforms/posix/posixSocket.cpp

Lines 484 to 495 in fc69321

	if (m_timeoutHandler && m_timeoutHandler->isTimeOut()) {
	if (!m_timeoutHandler->handleTimeOut()) {
	throw exceptions::operation_timed_out();
	} else {
	// Reset timeout and keep waiting for connection
	m_timeoutHandler->resetTimeOut();
	}
	}

If a timeout exception is thrown while getaddrinfo_a is still working, that is gai_error returns EAI_INPROGRESS, all local values that are directly or indirectly passed to getaddrinfo_a, like gaiRequest, address, portStr or hints, are destroyed. Subsequently, getaddrinfo_a may access invalid or repurposed memory. Unfortunately, gai_cancel can't cancel a request that is currently being processed (s. https://linux.die.net/man/3/gai_cancel).

This is an abridged reproduction of posixSocket::resolve:

#include <string>
#include <iostream>
#include <chrono>
#include <cstring>
#include <ctime>
#include <thread>
#include <netdb.h>

namespace {
	bool resolve(const std::string& address, unsigned int port, addrinfo** addrInfo) {
		gaicb gaiRequest;
		addrinfo hints;
		std::string portStr(std::to_string(port));

		std::memset(&hints, 0, sizeof(hints));

		hints.ai_flags = AI_CANONNAME | AI_NUMERICSERV;
		hints.ai_family = PF_UNSPEC;
		hints.ai_socktype = SOCK_STREAM;

		std::memset(&gaiRequest, 0, sizeof(gaiRequest));

		gaiRequest.ar_name = address.c_str();
		gaiRequest.ar_service = portStr.c_str();
		gaiRequest.ar_request = &hints;

		gaicb* gaiRequests = &gaiRequest;
		int gaiError;

		if ((gaiError = getaddrinfo_a(GAI_NOWAIT, &gaiRequests, 1, NULL)) != 0) {
			std::cerr << "getaddrinfo_a() failed: " << gai_strerror(gaiError) << std::endl;

			return false;
		}

		std::chrono::time_point<std::chrono::steady_clock> start = std::chrono::steady_clock::now();
		auto handler_timeout = std::chrono::milliseconds(1);

		while (true) {
			timespec gaiTimeout;

			gaiTimeout.tv_sec = 0;
			gaiTimeout.tv_nsec = 1000;

			gaiError = gai_suspend(&gaiRequests, 1, &gaiTimeout);

			if (gaiError == 0 || gaiError == EAI_ALLDONE) {
				const int ret = gai_error(&gaiRequest);

				if (ret != 0) {
					std::cerr << "getaddrinfo_a() request failed: " << gai_strerror(ret) << std::endl;

					return false;
				}
				else {
					*addrInfo = gaiRequest.ar_result;
					break;
				}
			}
			else if (gaiError != EAI_AGAIN) {
				if (gaiError == EAI_SYSTEM) {
					const int ret = gai_error(&gaiRequest);

					if (ret != EAI_INPROGRESS && errno != 0) {
						std::cerr << "Error while connecting socket." << std::endl;

						return false;
					}

				}
				else {
					std::cerr << "gai_suspend() failed: " << gai_strerror(gaiError) << std::endl;

					return false;
				}
			}

			if ((std::chrono::steady_clock::now() - start) >= handler_timeout) {
				std::cerr << "timeout" << std::endl;

				return false;
			}
		}

		return true;
	}
}

int main(int argc, char** argv) {
	{
		std::string address("smtp.office365.com");
		unsigned int port = 587;
		addrinfo* addrInfo = nullptr;

		if (resolve(address, port, &addrInfo)) {
			std::cout << "resolved" << std::endl;
		}

		freeaddrinfo(addrInfo);
	}

	// At this point, address, addrInfo and resolve' stack variables are all gone.

	// Wait so that getaddrinfo_a has a chance to finish.
	std::this_thread::sleep_for(std::chrono::seconds(5));

	return 0;
}

You may have to lower handler_timeout to reproduce the problem. Or you just return after getaddrinfo_a is called:

#include <string>
#include <iostream>
#include <chrono>
#include <cstring>
#include <ctime>
#include <thread>
#include <netdb.h>

namespace {
	bool resolve(const std::string& address, unsigned int port, addrinfo** addrInfo) {
		gaicb gaiRequest;
		addrinfo hints;
		std::string portStr(std::to_string(port));

		std::memset(&hints, 0, sizeof(hints));

		hints.ai_flags = AI_CANONNAME | AI_NUMERICSERV;
		hints.ai_family = PF_UNSPEC;
		hints.ai_socktype = SOCK_STREAM;

		std::memset(&gaiRequest, 0, sizeof(gaiRequest));

		gaiRequest.ar_name = address.c_str();
		gaiRequest.ar_service = portStr.c_str();
		gaiRequest.ar_request = &hints;

		gaicb* gaiRequests = &gaiRequest;
		int gaiError;

		if ((gaiError = getaddrinfo_a(GAI_NOWAIT, &gaiRequests, 1, NULL)) != 0) {
			std::cerr << "getaddrinfo_a() failed: " << gai_strerror(gaiError) << std::endl;

			return false;
		}

		return false;
	}
}

int main(int argc, char** argv) {
	{
		std::string address("smtp.office365.com");
		unsigned int port = 587;
		addrinfo* addrInfo = nullptr;

		if (resolve(address, port, &addrInfo)) {
			std::cout << "resolved" << std::endl;
		}

		freeaddrinfo(addrInfo);
	}

	// At this point, address, addrInfo and resolve' stack variables are all gone.

	// Wait so that getaddrinfo_a has a chance to finish.
	std::this_thread::sleep_for(std::chrono::seconds(5));

	return 0;
}

Since getaddrinfo_a basically creates a thread to call getaddrinfo, you can do it yourself like:

#include <string>
#include <iostream>
#include <chrono>
#include <cstring>
#include <ctime>
#include <thread>
#include <future>
#include <memory>
#include <netdb.h>

namespace {
	struct AddrinfoDeleter {
		void operator()(addrinfo** addrInfo) const {
			std::cout << "freeaddrinfo" << std::endl;

			if (addrInfo != nullptr) {
				freeaddrinfo(*addrInfo);
			}
		}
	};

	void resolve_thread_func(std::string address, unsigned int port, std::shared_ptr<addrinfo*> addrInfoPtr, std::promise<int> result) {
		addrinfo hints;

		std::memset(&hints, 0, sizeof(hints));

		hints.ai_flags = AI_CANONNAME | AI_NUMERICSERV;
		hints.ai_family = PF_UNSPEC;
		hints.ai_socktype = SOCK_STREAM;

		result.set_value(getaddrinfo(address.c_str(), std::to_string(port).c_str(), &hints, &*addrInfoPtr));
	}

	bool resolve(const std::string& address, unsigned int port, std::shared_ptr<addrinfo*> addrInfoPtr) {
		std::chrono::time_point<std::chrono::steady_clock> start = std::chrono::steady_clock::now();
		std::promise<int> resolve_promise;
		std::future<int> resolve_future(resolve_promise.get_future());
		std::thread resolve_thread(&resolve_thread_func, address, port, addrInfoPtr, std::move(resolve_promise));
		auto wait_timeout = std::chrono::nanoseconds(1000);
		auto handler_timeout = std::chrono::milliseconds(1);

		resolve_thread.detach();

		while (true) {
			switch (resolve_future.wait_for(wait_timeout)) {
				case std::future_status::deferred:
				case std::future_status::timeout:
					if ((std::chrono::steady_clock::now() - start) >= handler_timeout) {
						std::cerr << "timeout" << std::endl;

						return false;
					}

					break;

				case std::future_status::ready: {
					int result = resolve_future.get();

					std::cout << "getaddrinfo returned " << result << std::endl;

					return result == 0;
				}
			}
		}

		return false;
	}
}

int main(int argc, char** argv) {
	{
		std::string address("smtp.office365.com");
		unsigned int port = 587;
		addrinfo* addrInfo = nullptr;
		std::shared_ptr<addrinfo*> addrInfoPtr(&addrInfo, AddrinfoDeleter());

		if (resolve(address, port, addrInfoPtr)) {
			std::cout << "resolved" << std::endl;
		}
	}

	std::this_thread::sleep_for(std::chrono::seconds(5));

	return 0;
}

The idea is that the thread has its own copies of all necessary data and to wrap addrinfo* into a shared_ptr so that it gets freed once the thread, as it is detached, and all local sites are done with it.