Callback funcitons have incorrect signatures and cause UNDEFINED BEHAVIOUR

[ivan-r-sigaev]

The problem

curl::curl_easy::add<CURLOPT_HEADERFUNCTION>() and curl::curl_easy::add<CURLOPT_WRITEFUNCTION>() expect a function pointer of type size_t(*)(void *, size_t, size_t, void *)) instead of size_t(*)(char *, size_t, size_t, void *)) as libcurl requires (see CURLOPT_HEADERFUNCTION, CURLOPT_WRITEFUNCTION). The type mismatch may cause UB-related bugs down the line.

Why it happens

curl::detail::Option_type<Opt> with the Opt argument value of CURLOPT_HEADERFUNCTION or CURLOPT_WRITEFUNCTION resolves to size_t(*)(void *, size_t, size_t, void *)).

curlcpp/include/curl_easy.h

Lines 363 to 366 in be483f4

	/* Function that will be called to store headers (instead of fwrite). The
	* parameters will use fwrite() syntax, make sure to follow them. */
	CURLCPP_DEFINE_OPTION(CURLOPT_HEADERFUNCTION,
	size_t(*)(void *buffer, size_t size, size_t nitems, void *userdata));

curlcpp/include/curl_easy.h

Lines 172 to 175 in be483f4

	/* Function that will be called to store the output (instead of fwrite). The
	* parameters will use fwrite() syntax, make sure to follow them. */
	CURLCPP_DEFINE_OPTION(CURLOPT_WRITEFUNCTION,
	size_t(*)(void *ptr, size_t size, size_t nmemb, void *userdata));

This causes a problem down the line when the values get passed into the libcrul API:

curlcpp/include/curl_easy.h

Lines 1056 to 1061 in be483f4

	template <CURLoption Opt> void curl_easy::add(detail::Option_type<Opt> val) {
	const auto code = curl_easy_setopt(this->curl, Opt, val);
	if (code != CURLE_OK) {
	throw curl_easy_exception(code, __FUNCTION__);
	}
	}

Here curl_easy_setopt() expects a function pointer of type size_t(*)(char *, size_t, size_t, void *)) (see CURLOPT_HEADERFUNCTION, CURLOPT_WRITEFUNCTION), but gets size_t(*)(void *, size_t, size_t, void *)) instead.

According to cppreference's documentation on variadic functions:

If the type of the next argument in ap (after promotions) is not compatible with T, the behavior is undefined, unless:

• one type is a signed integer type, the other type is the corresponding unsigned integer type, and the value is representable in both types; or
• one type is pointer to void and the other is a pointer to a character type (char, signed char, or unsigned char).

The last line sound promising, but it only applies to raw pointers, not pointers inside function signatures.

Rules for type compatibility between two functions are:

The types T and U are compatible, if:

• they are function types, and:
  • their return types are compatible
  • they both use parameter lists, the number of parameters (including the use of the ellipsis) is the same, and the corresponding parameter, after applying array-to-pointer and function-to-pointer type adjustments and after stripping top-level qualifiers, have compatible types

Finally, void * and char * are incompatible because char and void are incompatible:

The types T and U are compatible, if

• they are pointer types and are pointing to compatible types

Therefore, size_t(*)(char *, size_t, size_t, void *)) and size_t(*)(void *, size_t, size_t, void *)) are incompatible. Therefore passing incorrect type to a variadic function results in UNDEFINED BEHAVIOUR.

Note: the code seems to work fine for me, but it is a potential breakage point in the future. Silent UB may create difficult to reproduce and deadly bugs for library users.