From e9b9bbac22c26cf67316fa8e6c6b9e831af31949 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 15 Nov 2024 11:06:36 +0100 Subject: [PATCH] netrc: address several netrc parser flaws - make sure that a match that returns a username also returns a password, that should be blank if no password is found - fix handling of multiple logins for same host where the password/login order might be reversed. - reject credentials provided in the .netrc if they contain ASCII control codes - if the used protocol does not support such (like HTTP and WS do) Reported-by: Harry Sintonen Add test 478, 479 and 480 to verify. Updated unit 1304. Closes #15586 Conflict:context adapt remove some code irrelevant with CVE Reference:https://github.com/curl/curl/commit/e9b9bbac22c26cf67316fa8e6c6b9e831af31949 --- lib/netrc.c | 19 ++++++++++++++++++ tests/data/Makefile.inc | 1 + tests/data/test479 | 107 ++++++++++++++++++++++++++++++++++++++ tests/unit/unit1304.c | 75 ++++++++------------------- 3 files changed, 180 insertions(+), 20 deletions(-) create mode 100644 tests/data/test478 create mode 100644 tests/data/test479 create mode 100644 tests/data/test480 diff --git a/lib/netrc.c b/lib/netrc.c index b771b60..9e183a2 100644 --- a/lib/netrc.c +++ b/lib/netrc.c @@ -152,6 +152,7 @@ static int parsenetrc(const char *host, retcode = NETRC_FAILED; /* allocation failed */ goto out; } + state_our_login = TRUE; login_alloc = TRUE; } state_login = 0; @@ -181,6 +182,16 @@ static int parsenetrc(const char *host, state = HOSTFOUND; state_our_login = FALSE; } + else if(strcasecompare("default", tok)) { + state = HOSTVALID; + retcode = NETRC_SUCCESS; /* we did find our host */ + Curl_safefree(password); + if(!specific_login) + if(login_alloc) { + free(login); + login_alloc = FALSE; + } + } break; } /* switch (state) */ @@ -189,6 +200,14 @@ static int parsenetrc(const char *host, } /* while fgets() */ out: + if(!retcode && !password && state_our_login) { + /* success without a password, set a blank one */ + password = strdup(""); + if(!password) + retcode = 1; /* out of memory */ + else + password_alloc = TRUE; + } if(!retcode) { /* success */ *login_changed = FALSE; diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index ea5221c00fd419..53f62c6e28f650 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -77,6 +77,7 @@ test435 test436 test437 test438 test439 test440 test441 test442 test443 \ test430 test431 test432 test433 test434 test435 test445 test446\ \ test442 test443 test444 \ +test479 \ test490 test491 test492 test493 test494 \ \ test500 test501 test502 test503 test504 test505 test506 test507 test508 \ diff --git a/tests/data/test479 b/tests/data/test479 new file mode 100644 index 00000000000000..d7ce4652fae272 --- /dev/null +++ b/tests/data/test479 @@ -0,0 +1,107 @@ + + + +netrc +HTTP + + +# +# Server-side + + +HTTP/1.1 301 Follow this you fool +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT +ETag: "21025-dc7-39462498" +Accept-Ranges: bytes +Content-Length: 6 +Connection: close +Location: http://b.com/%TESTNUMBER0002 + +-foo- + + + +HTTP/1.1 200 OK +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT +ETag: "21025-dc7-39462498" +Accept-Ranges: bytes +Content-Length: 7 +Connection: close + +target + + + +HTTP/1.1 301 Follow this you fool +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT +ETag: "21025-dc7-39462498" +Accept-Ranges: bytes +Content-Length: 6 +Connection: close +Location: http://b.com/%TESTNUMBER0002 + +HTTP/1.1 200 OK +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT +ETag: "21025-dc7-39462498" +Accept-Ranges: bytes +Content-Length: 7 +Connection: close + +target + + + +# +# Client-side + + +http + + +proxy + + +.netrc with redirect and default without password + + +--netrc --netrc-file log/netrc%TESTNUMBER -L -x http://%HOSTIP:%HTTPPORT/ http://a.com/ + + + +machine a.com + login alice + password alicespassword + +default + login bob + + + + + + +GET http://a.com/ HTTP/1.1 +Host: a.com +Authorization: Basic %b64[alice:alicespassword]b64% +User-Agent: curl/%VERSION +Accept: */* +Proxy-Connection: Keep-Alive + +GET http://b.com/%TESTNUMBER0002 HTTP/1.1 +Host: b.com +Authorization: Basic %b64[bob:]b64% +User-Agent: curl/%VERSION +Accept: */* +Proxy-Connection: Keep-Alive + + + + diff --git a/tests/unit/unit1304.c b/tests/unit/unit1304.c index 238d3c0f7fb09a..817887b94c8748 100644 --- a/tests/unit/unit1304.c +++ b/tests/unit/unit1304.c @@ -32,13 +32,8 @@ static char *password; static CURLcode unit_setup(void) { - password = strdup(""); - login = strdup(""); - if(!password || !login) { - Curl_safefree(password); - Curl_safefree(login); - return CURLE_OUT_OF_MEMORY; - } + password = NULL; + login = NULL; return CURLE_OK; } @@ -60,86 +55,52 @@ UNITTEST_START result = Curl_parsenetrc("test.example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 1, "Host not found should return 1"); - abort_unless(password != NULL, "returned NULL!"); - fail_unless(password[0] == 0, "password should not have been changed"); - abort_unless(login != NULL, "returned NULL!"); - fail_unless(login[0] == 0, "login should not have been changed"); + abort_unless(password == NULL, "password did not return NULL!"); + abort_unless(login == NULL, "user did not return NULL!"); /* * Test a non existent login in our netrc file. */ - free(login); - login = strdup("me"); - abort_unless(login != NULL, "returned NULL!"); + login = (char *)"me"; result = Curl_parsenetrc("example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found"); - abort_unless(password != NULL, "returned NULL!"); - fail_unless(password[0] == 0, "password should not have been changed"); - fail_unless(!password_changed, "password should not have been changed"); - abort_unless(login != NULL, "returned NULL!"); - fail_unless(strncmp(login, "me", 2) == 0, - "login should not have been changed"); - fail_unless(!login_changed, "login should not have been changed"); + abort_unless(password == NULL, "password is not NULL!"); /* * Test a non existent login and host in our netrc file. */ - free(login); - login = strdup("me"); - abort_unless(login != NULL, "returned NULL!"); + login = (char *)"me"; result = Curl_parsenetrc("test.example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 1, "Host not found should return 1"); - abort_unless(password != NULL, "returned NULL!"); - fail_unless(password[0] == 0, "password should not have been changed"); - abort_unless(login != NULL, "returned NULL!"); - fail_unless(strncmp(login, "me", 2) == 0, - "login should not have been changed"); + abort_unless(password == NULL, "password is not NULL!"); /* * Test a non existent login (substring of an existing one) in our * netrc file. */ - free(login); - login = strdup("admi"); - abort_unless(login != NULL, "returned NULL!"); + login = (char *)"admi"; result = Curl_parsenetrc("example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found"); - abort_unless(password != NULL, "returned NULL!"); - fail_unless(password[0] == 0, "password should not have been changed"); - fail_unless(!password_changed, "password should not have been changed"); - abort_unless(login != NULL, "returned NULL!"); - fail_unless(strncmp(login, "admi", 4) == 0, - "login should not have been changed"); - fail_unless(!login_changed, "login should not have been changed"); + abort_unless(password == NULL, "password is not NULL!"); /* * Test a non existent login (superstring of an existing one) * in our netrc file. */ - free(login); - login = strdup("adminn"); - abort_unless(login != NULL, "returned NULL!"); + login = (char *)"adminn"; result = Curl_parsenetrc("example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found"); - abort_unless(password != NULL, "returned NULL!"); - fail_unless(password[0] == 0, "password should not have been changed"); - fail_unless(!password_changed, "password should not have been changed"); - abort_unless(login != NULL, "returned NULL!"); - fail_unless(strncmp(login, "adminn", 6) == 0, - "login should not have been changed"); - fail_unless(!login_changed, "login should not have been changed"); + abort_unless(password == NULL, "password is not NULL!"); /* * Test for the first existing host in our netrc file * with login[0] = 0. */ - free(login); - login = strdup(""); - abort_unless(login != NULL, "returned NULL!"); + login = NULL; result = Curl_parsenetrc("example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found"); @@ -159,8 +126,9 @@ UNITTEST_START * with login[0] != 0. */ free(password); - password = strdup(""); - abort_unless(password != NULL, "returned NULL!"); + free(login); + password = NULL; + login = NULL; result = Curl_parsenetrc("example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found"); @@ -177,11 +145,9 @@ UNITTEST_START * with login[0] = 0. */ free(password); - password = strdup(""); - abort_unless(password != NULL, "returned NULL!"); + password = NULL; free(login); - login = strdup(""); - abort_unless(login != NULL, "returned NULL!"); + login = NULL; result = Curl_parsenetrc("curl.example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found"); @@ -198,8 +164,9 @@ UNITTEST_START * with login[0] != 0. */ free(password); - password = strdup(""); - abort_unless(password != NULL, "returned NULL!"); + free(login); + password = NULL; + login = NULL; result = Curl_parsenetrc("curl.example.com", &login, &password, &login_changed, &password_changed, filename); fail_unless(result == 0, "Host should have been found");