summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Kalnischkies <david@kalnischkies.de>2020-07-10 00:02:25 +0200
committerDavid Kalnischkies <david@kalnischkies.de>2020-12-18 19:31:19 +0100
commit97d6c3b2d05fe0d965657197adf56cc78f9edf81 (patch)
tree3c5b7c22635ffcde0b174e60660c01168e33ff71
parente6c55283d235aa9404395d30f2db891f36995c49 (diff)
Implement encoded URI handling in all methods
Every method opts in to getting the encoded URI passed along while keeping compat in case we are operated by an older acquire system. Effectively this is just a change for the http-based methods as the others just decode the URI as they work with files directly.
-rw-r--r--methods/aptmethod.h12
-rw-r--r--methods/basehttp.cc22
-rw-r--r--methods/cdrom.cc4
-rw-r--r--methods/copy.cc4
-rw-r--r--methods/file.cc4
-rw-r--r--methods/ftp.cc10
-rw-r--r--methods/gpgv.cc4
-rw-r--r--methods/http.cc7
-rw-r--r--methods/mirror.cc2
-rw-r--r--methods/rred.cc4
-rw-r--r--methods/rsh.cc10
-rw-r--r--methods/store.cc4
-rwxr-xr-xtest/integration/test-cve-2019-3462-dequote-injection27
13 files changed, 77 insertions, 37 deletions
diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index 67d5a3a0b..7038131cf 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -475,6 +475,18 @@ protected:
QueueBack = Queue;
delete Tmp;
}
+ static std::string URIEncode(std::string const &part)
+ {
+ // The "+" is encoded as a workaround for an S3 bug (LP#1003633 and LP#1086997)
+ return QuoteString(part, _config->Find("Acquire::URIEncode", "+~ ").c_str());
+ }
+
+ static std::string DecodeSendURI(std::string const &part)
+ {
+ if (_config->FindB("Acquire::Send-URI-Encoded", false))
+ return DeQuoteString(part);
+ return part;
+ }
aptMethod(std::string &&Binary, char const *const Ver, unsigned long const Flags) APT_NONNULL(3)
: pkgAcqMethod(Ver, Flags), Binary(Binary), SeccompFlags(0), methodNames({Binary})
diff --git a/methods/basehttp.cc b/methods/basehttp.cc
index b8ab73155..8aac1090c 100644
--- a/methods/basehttp.cc
+++ b/methods/basehttp.cc
@@ -283,6 +283,14 @@ void ServerState::Reset() /*{{{*/
/* We look at the header data we got back from the server and decide what
to do. Returns DealWithHeadersResult (see http.h for details).
*/
+static std::string fixURIEncoding(std::string const &part)
+{
+ // if the server sends a space this is not an encoded URI
+ // so other clients seem to encode it and we do it as well
+ if (part.find_first_of(" ") != std::string::npos)
+ return aptMethod::URIEncode(part);
+ return part;
+}
BaseHttpMethod::DealWithHeadersResult
BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
{
@@ -318,7 +326,10 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
NextURI = URI::SiteOnly(Uri);
else
NextURI.clear();
- NextURI.append(DeQuoteString(Req.Location));
+ if (_config->FindB("Acquire::Send-URI-Encoded", false))
+ NextURI.append(fixURIEncoding(Req.Location));
+ else
+ NextURI.append(DeQuoteString(Req.Location));
if (Queue->Uri == NextURI)
{
SetFailReason("RedirectionLoop");
@@ -331,8 +342,12 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
}
else
{
- NextURI = DeQuoteString(Req.Location);
- URI tmpURI(NextURI);
+ bool const SendURIEncoded = _config->FindB("Acquire::Send-URI-Encoded", false);
+ if (not SendURIEncoded)
+ Req.Location = DeQuoteString(Req.Location);
+ URI tmpURI(Req.Location);
+ if (SendURIEncoded)
+ tmpURI.Path = fixURIEncoding(tmpURI.Path);
if (tmpURI.Access.find('+') != std::string::npos)
{
_error->Error("Server tried to trick us into using a specific implementation: %s", tmpURI.Access.c_str());
@@ -340,6 +355,7 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
return ERROR_WITH_CONTENT_PAGE;
return ERROR_UNRECOVERABLE;
}
+ NextURI = tmpURI;
URI Uri(Queue->Uri);
if (Binary.find('+') != std::string::npos)
{
diff --git a/methods/cdrom.cc b/methods/cdrom.cc
index d836e1315..f534a06f5 100644
--- a/methods/cdrom.cc
+++ b/methods/cdrom.cc
@@ -57,7 +57,7 @@ class CDROMMethod : public aptMethod
/* */
CDROMMethod::CDROMMethod() : aptMethod("cdrom", "1.0",SingleInstance | LocalOnly |
SendConfig | NeedsCleanup |
- Removable),
+ Removable | SendURIEncoded),
DatabaseLoaded(false),
Debug(false),
MountedByApt(false)
@@ -175,7 +175,7 @@ bool CDROMMethod::Fetch(FetchItem *Itm)
FetchResult Res;
URI Get(Itm->Uri);
- string File = Get.Path;
+ std::string const File = DecodeSendURI(Get.Path);
Debug = DebugEnabled();
if (Debug)
diff --git a/methods/copy.cc b/methods/copy.cc
index 9a8665446..82eed150c 100644
--- a/methods/copy.cc
+++ b/methods/copy.cc
@@ -29,7 +29,7 @@ class CopyMethod : public aptMethod
virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
public:
- CopyMethod() : aptMethod("copy", "1.0", SingleInstance | SendConfig)
+ CopyMethod() : aptMethod("copy", "1.0", SingleInstance | SendConfig | SendURIEncoded)
{
SeccompFlags = aptMethod::BASE;
}
@@ -41,7 +41,7 @@ class CopyMethod : public aptMethod
bool CopyMethod::Fetch(FetchItem *Itm)
{
// this ensures that relative paths work in copy
- std::string const File = Itm->Uri.substr(Itm->Uri.find(':')+1);
+ std::string const File = DecodeSendURI(Itm->Uri.substr(Itm->Uri.find(':')+1));
// Stat the file and send a start message
struct stat Buf;
diff --git a/methods/file.cc b/methods/file.cc
index 80e47f1ad..b2fe133f2 100644
--- a/methods/file.cc
+++ b/methods/file.cc
@@ -32,7 +32,7 @@ class FileMethod : public aptMethod
virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
public:
- FileMethod() : aptMethod("file", "1.0", SingleInstance | SendConfig | LocalOnly)
+ FileMethod() : aptMethod("file", "1.0", SingleInstance | SendConfig | LocalOnly | SendURIEncoded)
{
SeccompFlags = aptMethod::BASE;
}
@@ -44,7 +44,7 @@ class FileMethod : public aptMethod
bool FileMethod::Fetch(FetchItem *Itm)
{
URI Get(Itm->Uri);
- std::string File = Get.Path;
+ std::string const File = DecodeSendURI(Get.Path);
FetchResult Res;
if (Get.Host.empty() == false)
return _error->Error(_("Invalid URI, local URIS must not start with //"));
diff --git a/methods/ftp.cc b/methods/ftp.cc
index 98398341e..16236232a 100644
--- a/methods/ftp.cc
+++ b/methods/ftp.cc
@@ -988,7 +988,7 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume,
// FtpMethod::FtpMethod - Constructor /*{{{*/
// ---------------------------------------------------------------------
/* */
-FtpMethod::FtpMethod() : aptAuthConfMethod("ftp", "1.0", SendConfig)
+FtpMethod::FtpMethod() : aptAuthConfMethod("ftp", "1.0", SendConfig | SendURIEncoded)
{
SeccompFlags = aptMethod::BASE | aptMethod::NETWORK;
signal(SIGTERM,SigTerm);
@@ -1038,7 +1038,7 @@ bool FtpMethod::Configuration(string Message)
bool FtpMethod::Fetch(FetchItem *Itm)
{
URI Get(Itm->Uri);
- const char *File = Get.Path.c_str();
+ auto const File = DecodeSendURI(Get.Path);
FetchResult Res;
Res.Filename = Itm->DestFile;
Res.IMSHit = false;
@@ -1070,8 +1070,8 @@ bool FtpMethod::Fetch(FetchItem *Itm)
// Get the files information
Status(_("Query"));
unsigned long long Size;
- if (Server->Size(File,Size) == false ||
- Server->ModTime(File,FailTime) == false)
+ if (not Server->Size(File.c_str(), Size) ||
+ not Server->ModTime(File.c_str(), FailTime))
{
Fail(true);
return true;
@@ -1119,7 +1119,7 @@ bool FtpMethod::Fetch(FetchItem *Itm)
FailFd = Fd.Fd();
bool Missing;
- if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->MaximumSize,this) == false)
+ if (not Server->Get(File.c_str(), Fd, Res.ResumePoint, Hash, Missing, Itm->MaximumSize, this))
{
Fd.Close();
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 5597e7cff..08d030a17 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -123,7 +123,7 @@ class GPGVMethod : public aptMethod
protected:
virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE;
public:
- GPGVMethod() : aptMethod("gpgv", "1.1", SingleInstance | SendConfig){};
+ GPGVMethod() : aptMethod("gpgv", "1.1", SingleInstance | SendConfig | SendURIEncoded){};
};
static void PushEntryWithKeyID(std::vector<std::string> &Signers, char * const buffer, bool const Debug)
{
@@ -419,7 +419,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
{
URI const Get(Itm->Uri);
- string const Path = Get.Host + Get.Path; // To account for relative paths
+ std::string const Path = DecodeSendURI(Get.Host + Get.Path); // To account for relative paths
SignersStorage Signers;
std::vector<std::string> keyFpts, keyFiles;
diff --git a/methods/http.cc b/methods/http.cc
index 0ef695166..b6d754037 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -901,9 +901,8 @@ void HttpMethod::SendReq(FetchItem *Itm)
else
requesturi = Uri;
- // The "+" is encoded as a workaround for a amazon S3 bug
- // see LP bugs #1003633 and #1086997.
- requesturi = QuoteString(requesturi, "+~ ");
+ if (not _config->FindB("Acquire::Send-URI-Encoded", false))
+ requesturi = URIEncode(requesturi);
/* Build the request. No keep-alive is included as it is the default
in 1.1, can cause problems with proxies, and we are an HTTP/1.1
@@ -1022,7 +1021,7 @@ BaseHttpMethod::DealWithHeadersResult HttpMethod::DealWithHeaders(FetchResult &R
return FILE_IS_OPEN;
}
/*}}}*/
-HttpMethod::HttpMethod(std::string &&pProg) : BaseHttpMethod(std::move(pProg), "1.2", Pipeline | SendConfig) /*{{{*/
+HttpMethod::HttpMethod(std::string &&pProg) : BaseHttpMethod(std::move(pProg), "1.2", Pipeline | SendConfig | SendURIEncoded) /*{{{*/
{
SeccompFlags = aptMethod::BASE | aptMethod::NETWORK;
diff --git a/methods/mirror.cc b/methods/mirror.cc
index 3e382e497..5c4d341c2 100644
--- a/methods/mirror.cc
+++ b/methods/mirror.cc
@@ -97,7 +97,7 @@ class MirrorMethod : public aptMethod /*{{{*/
void DealWithPendingItems(std::vector<std::string> const &baseuris, MirrorListInfo const &info, FetchItem *const Itm, std::function<void()> handler);
public:
- explicit MirrorMethod(std::string &&pProg) : aptMethod(std::move(pProg), "2.0", SingleInstance | Pipeline | SendConfig | AuxRequests), genrng(clock())
+ explicit MirrorMethod(std::string &&pProg) : aptMethod(std::move(pProg), "2.0", SingleInstance | Pipeline | SendConfig | AuxRequests | SendURIEncoded), genrng(clock())
{
SeccompFlags = aptMethod::BASE | aptMethod::DIRECTORY;
}
diff --git a/methods/rred.cc b/methods/rred.cc
index 2164cd19e..981364a9e 100644
--- a/methods/rred.cc
+++ b/methods/rred.cc
@@ -604,7 +604,7 @@ class RredMethod : public aptMethod {
virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE {
Debug = DebugEnabled();
URI Get(Itm->Uri);
- std::string Path = Get.Host + Get.Path; // rred:/path - no host
+ std::string Path = DecodeSendURI(Get.Host + Get.Path); // rred:/path - no host
FetchResult Res;
Res.Filename = Itm->DestFile;
@@ -750,7 +750,7 @@ class RredMethod : public aptMethod {
}
public:
- RredMethod() : aptMethod("rred", "2.0", SendConfig), Debug(false)
+ RredMethod() : aptMethod("rred", "2.0", SendConfig | SendURIEncoded), Debug(false)
{
SeccompFlags = aptMethod::BASE | aptMethod::DIRECTORY;
}
diff --git a/methods/rsh.cc b/methods/rsh.cc
index cc42b43e7..09d72cbc5 100644
--- a/methods/rsh.cc
+++ b/methods/rsh.cc
@@ -382,7 +382,7 @@ bool RSHConn::Get(const char *Path,FileFd &To,unsigned long long Resume,
/*}}}*/
// RSHMethod::RSHMethod - Constructor /*{{{*/
-RSHMethod::RSHMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.0",SendConfig)
+RSHMethod::RSHMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.0",SendConfig | SendURIEncoded)
{
signal(SIGTERM,SigTerm);
signal(SIGINT,SigTerm);
@@ -434,7 +434,7 @@ void RSHMethod::SigTerm(int)
bool RSHMethod::Fetch(FetchItem *Itm)
{
URI Get(Itm->Uri);
- const char *File = Get.Path.c_str();
+ auto const File = DecodeSendURI(Get.Path);
FetchResult Res;
Res.Filename = Itm->DestFile;
Res.IMSHit = false;
@@ -458,8 +458,8 @@ bool RSHMethod::Fetch(FetchItem *Itm)
// Get the files information
unsigned long long Size;
- if (Server->Size(File,Size) == false ||
- Server->ModTime(File,FailTime) == false)
+ if (not Server->Size(File.c_str(), Size) ||
+ not Server->ModTime(File.c_str(), FailTime))
{
//Fail(true);
//_error->Error(_("File not found")); // Will be handled by Size
@@ -505,7 +505,7 @@ bool RSHMethod::Fetch(FetchItem *Itm)
FailFd = Fd.Fd();
bool Missing;
- if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Res.Size) == false)
+ if (not Server->Get(File.c_str(), Fd, Res.ResumePoint, Hash, Missing, Res.Size))
{
Fd.Close();
diff --git a/methods/store.cc b/methods/store.cc
index 1b0f07947..8ffc7b93d 100644
--- a/methods/store.cc
+++ b/methods/store.cc
@@ -36,7 +36,7 @@ class StoreMethod : public aptMethod
public:
- explicit StoreMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.2",SingleInstance | SendConfig)
+ explicit StoreMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.2",SingleInstance | SendConfig | SendURIEncoded)
{
SeccompFlags = aptMethod::BASE;
if (Binary != "store")
@@ -64,7 +64,7 @@ static bool OpenFileWithCompressorByName(FileFd &fileFd, std::string const &File
bool StoreMethod::Fetch(FetchItem *Itm) /*{{{*/
{
URI Get(Itm->Uri);
- std::string Path = Get.Host + Get.Path; // To account for relative paths
+ std::string Path = DecodeSendURI(Get.Host + Get.Path); // To account for relative paths
FetchResult Res;
Res.Filename = Itm->DestFile;
diff --git a/test/integration/test-cve-2019-3462-dequote-injection b/test/integration/test-cve-2019-3462-dequote-injection
index 74ab03ba5..23cef4fae 100755
--- a/test/integration/test-cve-2019-3462-dequote-injection
+++ b/test/integration/test-cve-2019-3462-dequote-injection
@@ -16,9 +16,16 @@ SHA256="DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"
changetowebserver
runwithbaduri() {
- webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "$1"
+ local BADURI="$1"
+ local ERRMSG="$2"
+ shift 2
+ local BADFETCH="http://localhost:${APTHTTPPORT}/pool/alpha_1_all.deb"
+ if [ "$#" = '0' ]; then
+ BADFETCH="http://localhost:${APTHTTPPORT}/pool/$BADURI"
+ fi
+ webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "$BADURI"
- 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...
Building dependency tree...
@@ -28,13 +35,19 @@ The following NEW packages will be installed:
Need to get 20.7 kB of archives.
After this operation, 11.3 kB of additional disk space will be used.
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
+ $ERRMSG
+E: Failed to fetch $BADFETCH $ERRMSG
+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"
+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" 'SECURITY: URL redirect target contains control characters, rejecting.' -o Acquire::Send-URI-Encoded=false
+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" 'SECURITY: URL redirect target contains control characters, rejecting.' -o Acquire::Send-URI-Encoded=false
+
+# without de- and reencoding, we just trigger an error in our webserver as it refuses URIs containing '//'
+rm -rf rootdir/var/lib/apt/lists
+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" '400 Bad Request'
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"
+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" '400 Bad Request'
# For reference, the following is the original reproducer/bug. It has
# been disabled using exit 0, as it will fail in fixed versions.