summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-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.