From e76968e20dd9ea48220f9c82df50764ab44d7d7c Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Tue, 10 Mar 2026 01:03:13 +0100 Subject: [PATCH] curl_get_line: fix potential infinite loop when filename is a directory Fix potential inifinite loop reading file content with `Curl_get_line()` when a filename passed via these options are pointing to a directory entry (on non-Windows): - `--alt-svc` / `CURLOPT_ALTSVC` - `-b` / `--cookie` / `CURLOPT_COOKIEFILE` - `--hsts` / `CURLOPT_HSTS` - `--netrc-file` / `CURLOPT_NETRC_FILE` Fix by checking for this condition and silently skipping such filename without attempting to read content. Add test 1713 to verify. Mention in cookie documentation as an accepted case, also show a verbose message when a directory is detected. Extend test 46 to verify if such failure lets the logic continue to the next cookie file. Reported-and-based-on-patch-by: Richard Tollerton Fixes #20823 Closes #20826 (originally-based-on) Follow-up to 769ccb4d4261a75c8a4236fbe7dc3e27956db1c9 #19140 Closes #20873 --- docs/cmdline-opts/cookie.md | 3 +- docs/libcurl/opts/CURLOPT_COOKIEFILE.md | 2 + lib/altsvc.c | 29 +++++++------- lib/cookie.c | 13 ++++++- lib/hsts.c | 38 +++++++++--------- lib/netrc.c | 52 +++++++++++++------------ tests/data/Makefile.am | 2 +- tests/data/test1713 | 36 +++++++++++++++++ tests/data/test46 | 2 +- 9 files changed, 116 insertions(+), 61 deletions(-) create mode 100644 tests/data/test1713 diff --git a/docs/cmdline-opts/cookie.md b/docs/cmdline-opts/cookie.md index 50f977e70d..30288fbcba 100644 --- a/docs/cmdline-opts/cookie.md +++ b/docs/cmdline-opts/cookie.md @@ -54,7 +54,8 @@ the Netscape format. Users often want to both read cookies from a file and write updated cookies back to a file, so using both --cookie and --cookie-jar in the same command -line is common. +line is common. curl ignores filenames specified with --cookie which do not +exist or point to a directory. If curl is built with PSL (**Public Suffix List**) support, it detects and discards cookies that are specified for such suffix domains that should not be diff --git a/docs/libcurl/opts/CURLOPT_COOKIEFILE.md b/docs/libcurl/opts/CURLOPT_COOKIEFILE.md index 36403f9a4d..bf2d374464 100644 --- a/docs/libcurl/opts/CURLOPT_COOKIEFILE.md +++ b/docs/libcurl/opts/CURLOPT_COOKIEFILE.md @@ -58,6 +58,8 @@ list of files to read cookies from. The cookies are loaded from the specified file(s) when the transfer starts, not when this option is set. +libcurl ignores filenames which do not exist or point to a directory. + # SECURITY CONCERNS This document previously mentioned how specifying a non-existing file can also diff --git a/lib/altsvc.c b/lib/altsvc.c index 34da3e1ac8..20705d1159 100644 --- a/lib/altsvc.c +++ b/lib/altsvc.c @@ -208,19 +208,22 @@ static CURLcode altsvc_load(struct altsvcinfo *asi, const char *file) fp = curlx_fopen(file, FOPEN_READTEXT); if(fp) { - bool eof = FALSE; - struct dynbuf buf; - curlx_dyn_init(&buf, MAX_ALTSVC_LINE); - do { - result = Curl_get_line(&buf, fp, &eof); - if(!result) { - const char *lineptr = curlx_dyn_ptr(&buf); - curlx_str_passblanks(&lineptr); - if(curlx_str_single(&lineptr, '#')) - altsvc_add(asi, lineptr); - } - } while(!result && !eof); - curlx_dyn_free(&buf); /* free the line buffer */ + curlx_struct_stat stat; + if((curlx_fstat(fileno(fp), &stat) == -1) || !S_ISDIR(stat.st_mode)) { + bool eof = FALSE; + struct dynbuf buf; + curlx_dyn_init(&buf, MAX_ALTSVC_LINE); + do { + result = Curl_get_line(&buf, fp, &eof); + if(!result) { + const char *lineptr = curlx_dyn_ptr(&buf); + curlx_str_passblanks(&lineptr); + if(curlx_str_single(&lineptr, '#')) + altsvc_add(asi, lineptr); + } + } while(!result && !eof); + curlx_dyn_free(&buf); /* free the line buffer */ + } curlx_fclose(fp); } return result; diff --git a/lib/cookie.c b/lib/cookie.c index d52fa78b3c..92f7935cca 100644 --- a/lib/cookie.c +++ b/lib/cookie.c @@ -1106,8 +1106,17 @@ static CURLcode cookie_load(struct Curl_easy *data, const char *file, fp = curlx_fopen(file, "rb"); if(!fp) infof(data, "WARNING: failed to open cookie file \"%s\"", file); - else - handle = fp; + else { + curlx_struct_stat stat; + if((curlx_fstat(fileno(fp), &stat) != -1) && S_ISDIR(stat.st_mode)) { + curlx_fclose(fp); + fp = NULL; + infof(data, "WARNING: cookie filename points to a directory: \"%s\"", + file); + } + else + handle = fp; + } } } diff --git a/lib/hsts.c b/lib/hsts.c index 03f51f8109..3585af422a 100644 --- a/lib/hsts.c +++ b/lib/hsts.c @@ -505,26 +505,28 @@ static CURLcode hsts_load(struct hsts *h, const char *file) fp = curlx_fopen(file, FOPEN_READTEXT); if(fp) { - struct dynbuf buf; - bool eof = FALSE; - curlx_dyn_init(&buf, MAX_HSTS_LINE); - do { - result = Curl_get_line(&buf, fp, &eof); - if(!result) { - const char *lineptr = curlx_dyn_ptr(&buf); - curlx_str_passblanks(&lineptr); + curlx_struct_stat stat; + if((curlx_fstat(fileno(fp), &stat) == -1) || !S_ISDIR(stat.st_mode)) { + struct dynbuf buf; + bool eof = FALSE; + curlx_dyn_init(&buf, MAX_HSTS_LINE); + do { + result = Curl_get_line(&buf, fp, &eof); + if(!result) { + const char *lineptr = curlx_dyn_ptr(&buf); + curlx_str_passblanks(&lineptr); - /* - * Skip empty or commented lines, since we know the line will have a - * trailing newline from Curl_get_line we can treat length 1 as empty. - */ - if((*lineptr == '#') || strlen(lineptr) <= 1) - continue; + /* Skip empty or commented lines, since we know the line will have + a trailing newline from Curl_get_line we can treat length 1 as + empty. */ + if((*lineptr == '#') || strlen(lineptr) <= 1) + continue; - hsts_add(h, lineptr); - } - } while(!result && !eof); - curlx_dyn_free(&buf); /* free the line buffer */ + hsts_add(h, lineptr); + } + } while(!result && !eof); + curlx_dyn_free(&buf); /* free the line buffer */ + } curlx_fclose(fp); } return result; diff --git a/lib/netrc.c b/lib/netrc.c index 1073cc5365..d88609e358 100644 --- a/lib/netrc.c +++ b/lib/netrc.c @@ -72,34 +72,36 @@ static NETRCcode file2memory(const char *filename, struct dynbuf *filebuf) { NETRCcode ret = NETRC_FILE_MISSING; /* if it cannot open the file */ FILE *file = curlx_fopen(filename, FOPEN_READTEXT); - struct dynbuf linebuf; - curlx_dyn_init(&linebuf, MAX_NETRC_LINE); if(file) { - CURLcode result = CURLE_OK; - bool eof; - ret = NETRC_OK; - do { - const char *line; - result = Curl_get_line(&linebuf, file, &eof); - if(!result) { - line = curlx_dyn_ptr(&linebuf); - /* skip comments on load */ - curlx_str_passblanks(&line); - if(*line == '#') - continue; - result = curlx_dyn_add(filebuf, line); - } - if(result) { - curlx_dyn_free(filebuf); - ret = curl2netrc(result); - break; - } - } while(!eof); - } - curlx_dyn_free(&linebuf); - if(file) + curlx_struct_stat stat; + if((curlx_fstat(fileno(file), &stat) == -1) || !S_ISDIR(stat.st_mode)) { + CURLcode result = CURLE_OK; + bool eof; + struct dynbuf linebuf; + curlx_dyn_init(&linebuf, MAX_NETRC_LINE); + ret = NETRC_OK; + do { + const char *line; + result = Curl_get_line(&linebuf, file, &eof); + if(!result) { + line = curlx_dyn_ptr(&linebuf); + /* skip comments on load */ + curlx_str_passblanks(&line); + if(*line == '#') + continue; + result = curlx_dyn_add(filebuf, line); + } + if(result) { + curlx_dyn_free(filebuf); + ret = curl2netrc(result); + break; + } + } while(!eof); + curlx_dyn_free(&linebuf); + } curlx_fclose(file); + } return ret; } diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 65846e52ed..52bee86739 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -229,7 +229,7 @@ test1670 test1671 \ test1680 test1681 test1682 test1683 \ \ test1700 test1701 test1702 test1703 test1704 test1705 test1706 test1707 \ -test1708 test1709 test1710 test1711 test1712 \ +test1708 test1709 test1710 test1711 test1712 test1713 \ \ test1800 test1801 test1802 test1847 test1848 test1849 test1850 \ \ diff --git a/tests/data/test1713 b/tests/data/test1713 new file mode 100644 index 0000000000..9727ff615f --- /dev/null +++ b/tests/data/test1713 @@ -0,0 +1,36 @@ + + + + +HTTP +HTTP GET +Alt-Svc +cookies +HSTS +netrc + + + +# Client-side + + +Filenames pointing to directory failing gracefully + + +http://invalid.invalid/%TESTNUMBER --alt-svc %LOGDIR --cookie %LOGDIR --hsts %LOGDIR --netrc-file %LOGDIR + + +alt-svc +cookies +HSTS +netrc + + + + +# 26 = CURLE_READ_ERROR + +26 + + + diff --git a/tests/data/test46 b/tests/data/test46 index 1b0c8e515d..b46b0e41a6 100644 --- a/tests/data/test46 +++ b/tests/data/test46 @@ -48,7 +48,7 @@ HTTP with bad domain name, get cookies and store in cookie jar TZ=GMT -domain..tld:%HTTPPORT/want/%TESTNUMBER --resolve domain..tld:%HTTPPORT:%HOSTIP -c %LOGDIR/jar%TESTNUMBER -b %LOGDIR/injar%TESTNUMBER +domain..tld:%HTTPPORT/want/%TESTNUMBER --resolve domain..tld:%HTTPPORT:%HOSTIP -c %LOGDIR/jar%TESTNUMBER -b %LOGDIR -b %LOGDIR/injar%TESTNUMBER # Netscape HTTP Cookie File