From 97be873d782c5e9aaa8b4f4f4e6e18805d0fa51c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 8 Jul 2020 17:51:40 +0200 Subject: Proper URI encoding for config requests to our test webserver Our http method encodes the URI again which results in the double encoding we have unwrap in the webserver (we did already, but we skip the filename handling now which does the first decode). --- cmdline/apt-helper.cc | 9 +++++++++ test/integration/framework | 5 ++--- test/integration/test-cve-2019-3462-dequote-injection | 15 ++++++++------- test/interactive-helper/aptwebserver.cc | 14 ++++++++++---- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/cmdline/apt-helper.cc b/cmdline/apt-helper.cc index 3d6a692e0..85795e0d2 100644 --- a/cmdline/apt-helper.cc +++ b/cmdline/apt-helper.cc @@ -284,6 +284,14 @@ static bool AnalyzePattern(CommandLine &CmdL) /*{{{*/ return false; } + return true; +} + /*}}}*/ +static bool DoQuoteString(CommandLine &CmdL) /*{{{*/ +{ + if (CmdL.FileSize() != 3) + return _error->Error("Expect two arguments, a string to quote and a string of additional characters to quote"); + std::cout << QuoteString(CmdL.FileList[1], CmdL.FileList[2]) << '\n'; return true; } /*}}}*/ @@ -310,6 +318,7 @@ static std::vector GetCommands() /*{{{*/ {"drop-privs", &DropPrivsAndRun, _("drop privileges before running given command")}, {"analyze-pattern", &AnalyzePattern, _("analyse a pattern")}, {"analyse-pattern", &AnalyzePattern, nullptr}, + {"quote-string", &DoQuoteString, nullptr}, {nullptr, nullptr, nullptr}}; } /*}}}*/ diff --git a/test/integration/framework b/test/integration/framework index e30fa066c..20173da23 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1327,14 +1327,13 @@ webserverconfig() { local DOWNLOG="${TMPWORKINGDIRECTORY}/rootdir/tmp/download-testfile.log" local STATUS="${TMPWORKINGDIRECTORY}/downloaded/webserverconfig.status" rm -f "$STATUS" "$DOWNLOG" - # very very basic URI encoding local URI if [ -n "$2" ]; then msgtest "Set webserver config option '${1}' to" "$2" - URI="${WEBSERVER}/_config/set/$(echo "${1}" | sed -e 's/\//%2f/g')/$(echo "${2}" | sed -e 's/\//%2f/g')" + URI="${WEBSERVER}/_config/set/$(apthelper quote-string "${1}" '/?#')/$(apthelper quote-string "${2}" '/?#')" else msgtest 'Clear webserver config option' "${1}" - URI="${WEBSERVER}/_config/clear/$(echo "${1}" | sed -e 's/\//%2f/g')" + URI="${WEBSERVER}/_config/clear/$(apthelper quote-string "${1}" '/?#')" fi if downloadfile "$URI" "$STATUS" > "$DOWNLOG"; then msgpass diff --git a/test/integration/test-cve-2019-3462-dequote-injection b/test/integration/test-cve-2019-3462-dequote-injection index a1adec6de..74ab03ba5 100755 --- a/test/integration/test-cve-2019-3462-dequote-injection +++ b/test/integration/test-cve-2019-3462-dequote-injection @@ -15,13 +15,12 @@ ORIGINAL_SIZE=$(wc -c aptarchive/pool/alpha_1_all.deb | awk '{print $1}') SHA256="DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" changetowebserver -webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a" +runwithbaduri() { + webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "$1" + testsuccess apt update -o debug::http=1 -o debug::pkgacquire::worker=1 -testsuccess apt update -o debug::http=1 -o debug::pkgacquire::worker=1 - - -testfailureequal "Reading package lists... + testfailureequal "Reading package lists... Building dependency tree... The following NEW packages will be installed: alpha @@ -32,8 +31,10 @@ Err:1 http://localhost:${APTHTTPPORT} unstable/main all alpha all 1 SECURITY: URL redirect target contains control characters, rejecting. E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/alpha_1_all.deb SECURITY: URL redirect target contains control characters, rejecting. E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?" aptget install alpha - - +} +runwithbaduri "beeta_1_all.deb%0a%0a201%20URI%20Done%0aURI:%20http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%0aFilename:%20${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%0aSize:%2020672%0aLast-Modified:%20Fri,%2018%20Jan%202019%2009:52:02%20+0000%0aSHA256-Hash:%20${SHA256}%0aChecksum-FileSize-Hash:%2012345%0a%0a%0a" +rm -rf rootdir/var/lib/apt/lists +runwithbaduri "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a" # For reference, the following is the original reproducer/bug. It has # been disabled using exit 0, as it will fail in fixed versions. diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index f074cd148..57d215a65 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -573,6 +573,11 @@ static bool parseFirstLine(std::ostream &log, int const client, std::string cons params = filename.substr(paramspos + 1); filename.erase(paramspos); } + else if (APT::String::Startswith(filename, "/_config/")) + { + filename.erase(0, 1); + return true; + } filename = DeQuoteString(filename); @@ -620,11 +625,12 @@ static bool parseFirstLine(std::ostream &log, int const client, std::string cons } /*}}}*/ static bool handleOnTheFlyReconfiguration(std::ostream &log, int const client,/*{{{*/ - std::string const &request, std::vector parts, std::list &headers) + std::string const &request, std::vector const &EncodedParts, std::list &headers) { - size_t const pcount = parts.size(); + size_t const pcount = EncodedParts.size(); + std::vector parts(pcount); for (size_t i = 0; i < pcount; ++i) - parts[i] = DeQuoteString(parts[i]); + parts[i] = DeQuoteString(DeQuoteString(EncodedParts[i])); if (pcount == 4 && parts[1] == "set") { _config->Set(parts[2], parts[3]); @@ -707,7 +713,7 @@ static void * handleClient(int const client, size_t const id) /*{{{*/ // special webserver command request if (filename.length() > 1 && filename[0] == '_') { - std::vector parts = VectorizeString(filename, '/'); + auto const parts = VectorizeString(filename, '/'); if (parts[0] == "_config") { handleOnTheFlyReconfiguration(log, client, *m, parts, headers); -- cgit v1.2.3