From 98e6629154044e4ab1ee7cff8351c7ebcb131e88 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 2 Jan 2019 20:18:27 +0100 Subject: xattr: strip credentials from any URL that is stored Both user and password are cleared uncondtitionally. Added unit test 1621 to verify. Fixes #3423 Closes #3433 --- src/tool_xattr.c | 63 +++++++++++++++++++++++++++++----- tests/data/Makefile.inc | 2 +- tests/data/test1621 | 27 +++++++++++++++ tests/unit/Makefile.inc | 6 +++- tests/unit/unit1621.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 177 insertions(+), 10 deletions(-) create mode 100644 tests/data/test1621 create mode 100644 tests/unit/unit1621.c diff --git a/src/tool_xattr.c b/src/tool_xattr.c index 92b99db60..730381ba9 100644 --- a/src/tool_xattr.c +++ b/src/tool_xattr.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2014, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -49,6 +49,46 @@ static const struct xattr_mapping { { NULL, CURLINFO_NONE } /* last element, abort loop here */ }; +/* returns TRUE if a new URL is returned, that then needs to be freed */ +/* @unittest: 1621 */ +#ifdef UNITTESTS +bool stripcredentials(char **url); +#else +static +#endif +bool stripcredentials(char **url) +{ + CURLU *u; + CURLUcode uc; + char *nurl; + u = curl_url(); + if(u) { + uc = curl_url_set(u, CURLUPART_URL, *url, 0); + if(uc) + goto error; + + uc = curl_url_set(u, CURLUPART_USER, NULL, 0); + if(uc) + goto error; + + uc = curl_url_set(u, CURLUPART_PASSWORD, NULL, 0); + if(uc) + goto error; + + uc = curl_url_get(u, CURLUPART_URL, &nurl, 0); + if(uc) + goto error; + + curl_url_cleanup(u); + + *url = nurl; + return TRUE; + } + error: + curl_url_cleanup(u); + return FALSE; +} + /* store metadata from the curl request alongside the downloaded * file using extended attributes */ @@ -62,17 +102,24 @@ int fwrite_xattr(CURL *curl, int fd) char *value = NULL; CURLcode result = curl_easy_getinfo(curl, mappings[i].info, &value); if(!result && value) { + bool freeptr = FALSE; + if(CURLINFO_EFFECTIVE_URL == mappings[i].info) + freeptr = stripcredentials(&value); + if(value) { #ifdef HAVE_FSETXATTR_6 - err = fsetxattr(fd, mappings[i].attr, value, strlen(value), 0, 0); + err = fsetxattr(fd, mappings[i].attr, value, strlen(value), 0, 0); #elif defined(HAVE_FSETXATTR_5) - err = fsetxattr(fd, mappings[i].attr, value, strlen(value), 0); + err = fsetxattr(fd, mappings[i].attr, value, strlen(value), 0); #elif defined(__FreeBSD_version) - err = extattr_set_fd(fd, EXTATTR_NAMESPACE_USER, mappings[i].attr, value, - strlen(value)); - /* FreeBSD's extattr_set_fd returns the length of the extended attribute - */ - err = err < 0 ? err : 0; + err = extattr_set_fd(fd, EXTATTR_NAMESPACE_USER, mappings[i].attr, + value, strlen(value)); + /* FreeBSD's extattr_set_fd returns the length of the extended + attribute */ + err = err < 0 ? err : 0; #endif + if(freeptr) + curl_free(value); + } } i++; } diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index 79bbc657d..85cf2755c 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -182,7 +182,7 @@ test1560 test1561 test1562 \ test1590 test1591 test1592 \ \ test1600 test1601 test1602 test1603 test1604 test1605 test1606 test1607 \ -test1608 test1609 test1620 \ +test1608 test1609 test1620 test1621 \ \ test1650 test1651 test1652 test1653 \ \ diff --git a/tests/data/test1621 b/tests/data/test1621 new file mode 100644 index 000000000..1117d1bd2 --- /dev/null +++ b/tests/data/test1621 @@ -0,0 +1,27 @@ + + + +unittest +stripcredentials + + + +# +# Client-side + + +none + + +unittest +https + + +unit tests for stripcredentials from URL + + +unit1621 + + + + diff --git a/tests/unit/Makefile.inc b/tests/unit/Makefile.inc index 8b1a6071a..82eaec797 100644 --- a/tests/unit/Makefile.inc +++ b/tests/unit/Makefile.inc @@ -10,7 +10,7 @@ UNITPROGS = unit1300 unit1301 unit1302 unit1303 unit1304 unit1305 unit1307 \ unit1330 unit1394 unit1395 unit1396 unit1397 unit1398 \ unit1399 \ unit1600 unit1601 unit1602 unit1603 unit1604 unit1605 unit1606 unit1607 \ - unit1608 unit1609 unit1620 \ + unit1608 unit1609 unit1620 unit1621 \ unit1650 unit1651 unit1652 unit1653 unit1300_SOURCES = unit1300.c $(UNITFILES) @@ -100,6 +100,10 @@ unit1609_CPPFLAGS = $(AM_CPPFLAGS) unit1620_SOURCES = unit1620.c $(UNITFILES) unit1620_CPPFLAGS = $(AM_CPPFLAGS) +unit1621_SOURCES = unit1621.c $(UNITFILES) +unit1621_CPPFLAGS = $(AM_CPPFLAGS) +unit1621_LDADD = $(top_builddir)/src/libcurltool.la $(top_builddir)/lib/libcurl.la + unit1650_SOURCES = unit1650.c $(UNITFILES) unit1650_CPPFLAGS = $(AM_CPPFLAGS) diff --git a/tests/unit/unit1621.c b/tests/unit/unit1621.c new file mode 100644 index 000000000..6e07b6ea9 --- /dev/null +++ b/tests/unit/unit1621.c @@ -0,0 +1,89 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at https://curl.haxx.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + ***************************************************************************/ +#include "curlcheck.h" + +#include "urldata.h" +#include "url.h" + +#include "memdebug.h" /* LAST include file */ + +static CURLcode unit_setup(void) +{ + return CURLE_OK; +} + +static void unit_stop(void) +{ +} + +#ifdef __MINGW32__ +UNITTEST_START +{ + return 0; +} +UNITTEST_STOP +#else + +bool stripcredentials(char **url); + +struct checkthis { + const char *input; + const char *output; +}; + +static struct checkthis tests[] = { + { "ninja://foo@example.com", "ninja://foo@example.com" }, + { "https://foo@example.com", "https://example.com/" }, + { "https://localhost:45", "https://localhost:45/" }, + { "https://foo@localhost:45", "https://localhost:45/" }, + { "http://daniel:password@localhost", "http://localhost/" }, + { "http://daniel@localhost", "http://localhost/" }, + { "http://localhost/", "http://localhost/" }, + { NULL, NULL } /* end marker */ +}; + +UNITTEST_START +{ + bool cleanup; + char *url; + int i; + int rc = 0; + + for(i = 0; tests[i].input; i++) { + url = (char *)tests[i].input; + cleanup = stripcredentials(&url); + printf("Test %u got input \"%s\", output: \"%s\"\n", + i, tests[i].input, url); + + if(strcmp(tests[i].output, url)) { + fprintf(stderr, "Test %u got input \"%s\", expected output \"%s\"\n" + " Actual output: \"%s\"\n", i, tests[i].input, tests[i].output, + url); + rc++; + } + if(cleanup) + curl_free(url); + } + return rc; +} +UNITTEST_STOP +#endif -- cgit v1.2.1