Signed Integer Overflow in tftp_set_timeouts via Curl_timeleft_ms Returning LONG_MAX

### I did this ## Summary A signed integer overflow in `tftp_set_timeouts` caused by `Curl_timeleft_ms()` returning `TIMEDIFF_T_MAX` (9223372036854775807), which when added to 500 at `tftp.c:171` overflows the `timediff_t` type. This is undefined behavior per the C standard and is detected by UBSAN. The overflow occurs when the TFTP protocol handler is invoked while `CURLOPT_TIMEOUT_MS` is set to a very large value (e.g., `LONG_MAX`), causing `Curl_timeleft_ms()` to return a value close to `TIMEDIFF_T_MAX`. ## Version afdd8f1290b418896db75e01df4cb60712a892c8 (curl 8.20.0-DEV, curl/curl) ## Description `tftp_set_timeouts()` calls `Curl_timeleft_ms()` to determine the remaining timeout in milliseconds (tftp.c:161). The function `Curl_timeleft_ms()` (connect.c:140) delegates to `Curl_timeleft_now_ms()`, which computes the remaining time as: ```c if(data->set.timeout) { timeleft_ms = data->set.timeout - curlx_ptimediff_ms(pnow, &data->progress.t_startop); } ``` When `data->set.timeout` is set to a very large value via `CURLOPT_TIMEOUT_MS`, and the elapsed time since `t_startop` is small, `timeleft_ms` can be close to `TIMEDIFF_T_MAX`. Specifically: - `CURLOPT_TIMEOUT_MS` accepts a `long` value. On 64-bit Linux, `LONG_MAX = 0x7FFFFFFFFFFFFFFF`. - `setopt_set_timeout_ms()` stores this directly as `data->set.timeout = (timediff_t)ms` (since `LONG_MAX <= TIMEDIFF_T_MAX` on 64-bit systems, the guard `#if LONG_MAX > TIMEDIFF_T_MAX` is false and no clamping occurs). - With near-zero elapsed time, `timeleft_ms = LONG_MAX - small_elapsed ≈ LONG_MAX`. At tftp.c:171, the code performs `timeout = (time_t)(timeout_ms + 500) / 1000;` to round the millisecond timeout to seconds. When `timeout_ms` is `TIMEDIFF_T_MAX` (9223372036854775807), adding 500 causes a signed integer overflow: ``` 9223372036854775807 + 500 → 0x80000000000001F3 (wraps to negative in signed long) ``` The subsequent division by 1000 produces `0xFFDF3B645A1CAC09`, an incorrect negative `time_t` value. This leads to wrong `retry_max` (line 176) and `retry_time` (line 186) calculations. The check at tftp.c:163 only handles `timeout_ms < 0` (timeout already elapsed), but does not check whether `timeout_ms` is close to `TIMEDIFF_T_MAX` before the addition. The bug is that `tftp_set_timeouts` assumes `timeout_ms + 500` will not overflow, but `Curl_timeleft_ms()` can legitimately return values near `TIMEDIFF_T_MAX`. ## PoC Code ```c #include <curl/curl.h> #include <unistd.h> #include <signal.h> #include <stdlib.h> #include <limits.h> /* Trigger input: a TFTP URL that activates the TFTP protocol handler. * Original fuzzer input (with switch_id prefix stripped): * "tfTP://0/10\x00\x00\x00\x00\x04\x00" */ const char trigger_url[] = "tftp://0/10"; void alarm_handler(int sig) { _exit(0); } int main() { signal(SIGALRM, alarm_handler); alarm(5); /* Prevent hanging on network ops */ curl_global_init(CURL_GLOBAL_ALL); CURL *curl = curl_easy_init(); if (!curl) return 1; /* Set a TFTP URL to trigger the TFTP protocol handler */ curl_easy_setopt(curl, CURLOPT_URL, trigger_url); /* Set timeout to LONG_MAX. setopt_set_timeout_ms() stores it as * data->set.timeout = (timediff_t)LONG_MAX (no clamping on 64-bit). * Curl_timeleft_now_ms() computes: * timeleft_ms = LONG_MAX - curlx_ptimediff_ms(pnow, &t_startop) * With small elapsed time, timeleft_ms ≈ LONG_MAX. * Then at tftp.c:171: timeout = (time_t)(timeout_ms + 500) / 1000; * LONG_MAX + 500 overflows signed timediff_t → undefined behavior. */ curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS, (long)LONG_MAX); /* This triggers tftp_set_timeouts() -> Curl_timeleft_ms() ≈ LONG_MAX * -> timeout_ms + 500 overflows at tftp.c:171 */ curl_easy_perform(curl); curl_easy_cleanup(curl); curl_global_cleanup(); return 0; } ``` ## Reproduction Step ```bash # Build against curl 8.20.0-DEV with UBSAN gcc -g -fsanitize=undefined -I<path-to-curl-include> -o poc poc.c \ <path-to-curl-build>/lib/.libs/libcurl.a -lz -lssl -lcrypto -lpthread # Run (alarm prevents hanging on network ops) timeout 10 ./poc # Expected UBSAN output: # ../../lib/tftp.c:171:35: runtime error: signed integer overflow: # 9223372036854775807 + 500 cannot be represented in type 'timediff_t' (aka 'long') ``` ## Original fuzzer stack trace: ``` ../../lib/tftp.c:171:35: runtime error: signed integer overflow: 9223372036854775807 + 500 cannot be represented in type 'timediff_t' (aka 'long') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../lib/tftp.c:171:35 #0 tftp_set_timeouts tftp.c:171 #1 tftp_connect tftp.c:957 #2 protocol_connect multi.c:1907 #3 multi_runsingle multi.c:2591 #4 multi_perform multi.c:2806 #5 easy_transfer easy.c:694 #6 easy_perform easy.c:802 #7 LLVMFuzzerTestOneInput_16 id_000016.cc:80 ``` ## Security Impact Low severity. The overflow is triggered when `Curl_timeleft_ms()` returns `TIMEDIFF_T_MAX`, which occurs when the application sets a very large `CURLOPT_TIMEOUT_MS` (e.g., `LONG_MAX`) while the TFTP protocol handler is active. The immediate consequence is undefined behavior (signed integer overflow), which can produce incorrect TFTP retry parameters. In practice, this is most likely to cause incorrect timeout behavior or, in the worst case, a loop with very large retry counts in TFTP transfers. Applications that accept user-controlled timeout values are potentially affected. The fix should clamp `timeout_ms` before the addition in `tftp.c:171`: ```c // Before: timeout = (time_t)(timeout_ms + 500) / 1000; // After (overflow-safe rounding): if(timeout_ms > TIMEDIFF_T_MAX - 500) timeout = (time_t)(timeout_ms / 1000); else timeout = (time_t)(timeout_ms + 500) / 1000; ``` Or alternatively, avoid the addition entirely with overflow-safe rounding: ```c timeout = (time_t)(timeout_ms / 1000) + (timeout_ms % 1000 >= 500 ? 1 : 0); ``` Signed-off-by: FuzzAnything fuzzanything@gmail.com ### I expected the following _No response_ ### curl/libcurl version curl 8.20.0 ### operating system Ubuntu 22.04.5 LTS