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).

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
@@ -287,6 +287,14 @@ static bool AnalyzePattern(CommandLine &CmdL)				/*{{{*/
    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;
+}
+									/*}}}*/
 static bool ShowHelp(CommandLine &)					/*{{{*/
 {
    std::cout <<
@@ -310,6 +318,7 @@ static std::vector<aptDispatchWithHelp> 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<std::string> parts, std::list<std::string> &headers)
+      std::string const &request, std::vector<std::string> const &EncodedParts, std::list<std::string> &headers)
 {
-   size_t const pcount = parts.size();
+   size_t const pcount = EncodedParts.size();
+   std::vector<std::string> 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<std::string> parts = VectorizeString(filename, '/');
+	    auto const parts = VectorizeString(filename, '/');
 	    if (parts[0] == "_config")
 	    {
 	       handleOnTheFlyReconfiguration(log, client, *m, parts, headers);