diff options
-rw-r--r-- | methods/aptmethod.h | 12 | ||||
-rw-r--r-- | methods/basehttp.cc | 22 | ||||
-rw-r--r-- | methods/cdrom.cc | 4 | ||||
-rw-r--r-- | methods/copy.cc | 4 | ||||
-rw-r--r-- | methods/file.cc | 4 | ||||
-rw-r--r-- | methods/ftp.cc | 10 | ||||
-rw-r--r-- | methods/gpgv.cc | 4 | ||||
-rw-r--r-- | methods/http.cc | 7 | ||||
-rw-r--r-- | methods/mirror.cc | 2 | ||||
-rw-r--r-- | methods/rred.cc | 4 | ||||
-rw-r--r-- | methods/rsh.cc | 10 | ||||
-rw-r--r-- | methods/store.cc | 4 | ||||
-rwxr-xr-x | test/integration/test-cve-2019-3462-dequote-injection | 27 |
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. |