vmime
vmime copied to clipboard
kisli
`posixSocket::resolve` may wreak havoc
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: https://github.com/kisli/vmime/blob/fc69321d5304c73be685c890f3b30528aadcfeaf/src/vmime/platforms/posix/posixSocket.cpp#L484-L495
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.
