From dcd5856b11c685ca6d4629212d2978ce196ea65c Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 26 Aug 2014 19:08:37 -0700 Subject: Pass ExpectedSize to tthe backend method This ensures that we can stop downloading if the server send too much data by accident (or by a malicious attempt) --- methods/http.cc | 10 +++++++++- methods/http.h | 4 +++- methods/server.cc | 5 +++++ methods/server.h | 4 +++- 4 files changed, 20 insertions(+), 3 deletions(-) (limited to 'methods') diff --git a/methods/http.cc b/methods/http.cc index c734d3799..916fa464f 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -63,7 +63,8 @@ const unsigned int CircleBuf::BW_HZ=10; // CircleBuf::CircleBuf - Circular input buffer /*{{{*/ // --------------------------------------------------------------------- /* */ -CircleBuf::CircleBuf(unsigned long long Size) : Size(Size), Hash(0) +CircleBuf::CircleBuf(unsigned long long Size) + : Size(Size), Hash(0), TotalWriten(0) { Buf = new unsigned char[Size]; Reset(); @@ -79,6 +80,7 @@ void CircleBuf::Reset() InP = 0; OutP = 0; StrPos = 0; + TotalWriten = 0; MaxGet = (unsigned long long)-1; OutQueue = string(); if (Hash != 0) @@ -216,6 +218,8 @@ bool CircleBuf::Write(int Fd) return false; } + + TotalWriten += Res; if (Hash != 0) Hash->Add(Buf + (OutP%Size),Res); @@ -649,6 +653,10 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File) return _error->Errno("write",_("Error writing to output file")); } + if (ExpectedSize > 0 && In.TotalWriten > ExpectedSize) + return _error->Error("Writing more data than expected (%llu > %llu)", + In.TotalWriten, ExpectedSize); + // Handle commands from APT if (FD_ISSET(STDIN_FILENO,&rfds)) { diff --git a/methods/http.h b/methods/http.h index 5406ce4a7..c98fe8e5f 100644 --- a/methods/http.h +++ b/methods/http.h @@ -63,6 +63,8 @@ class CircleBuf public: Hashes *Hash; + // total amount of data that got written so far + unsigned long long TotalWriten; // Read data in bool Read(int Fd); @@ -81,8 +83,8 @@ class CircleBuf bool ReadSpace() const {return Size - (InP - OutP) > 0;}; bool WriteSpace() const {return InP - OutP > 0;}; - // Dump everything void Reset(); + // Dump everything void Stats(); CircleBuf(unsigned long long Size); diff --git a/methods/server.cc b/methods/server.cc index c91d3b218..1b6511c59 100644 --- a/methods/server.cc +++ b/methods/server.cc @@ -531,6 +531,11 @@ int ServerMethod::Loop() // Run the data bool Result = true; + + // ensure we don't fetch too much + if (Queue->ExpectedSize > 0) + Server->ExpectedSize = Queue->ExpectedSize; + if (Server->HaveContent) Result = Server->RunData(File); diff --git a/methods/server.h b/methods/server.h index 5299b3954..0d7333140 100644 --- a/methods/server.h +++ b/methods/server.h @@ -49,6 +49,8 @@ struct ServerState URI Proxy; unsigned long TimeOut; + unsigned long long ExpectedSize; + protected: ServerMethod *Owner; @@ -73,7 +75,7 @@ struct ServerState bool Comp(URI Other) const {return Other.Host == ServerName.Host && Other.Port == ServerName.Port;}; virtual void Reset() {Major = 0; Minor = 0; Result = 0; Code[0] = '\0'; Size = 0; StartPos = 0; Encoding = Closes; time(&Date); HaveContent = false; - State = Header; Persistent = false; Pipeline = true;}; + State = Header; Persistent = false; Pipeline = true; ExpectedSize = 0;}; virtual bool WriteResponse(std::string const &Data) = 0; /** \brief Transfer the data from the socket */ -- cgit v1.2.3 From ffd2dd93a640b47663ebdccc4fda00b426b3db71 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 26 Aug 2014 19:20:04 -0700 Subject: make https honor ExpectedSize as well --- methods/https.cc | 6 ++++++ methods/https.h | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'methods') diff --git a/methods/https.cc b/methods/https.cc index e0348ab58..cacd8a6bc 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -81,6 +81,12 @@ HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp) if(me->File->Write(buffer, size*nmemb) != true) return false; + me->TotalWritten += size*nmemb; + if(me->TotalWritten > me->Queue->ExpectedSize) + return _error->Error("Writing more data than expected (%llu > %llu)", + me->TotalWritten, me->Queue->ExpectedSize); + + return size*nmemb; } diff --git a/methods/https.h b/methods/https.h index faac8a3cd..2335559fb 100644 --- a/methods/https.h +++ b/methods/https.h @@ -66,11 +66,12 @@ class HttpsMethod : public pkgAcqMethod CURL *curl; FetchResult Res; HttpsServerState *Server; + unsigned long long TotalWritten; public: FileFd *File; - HttpsMethod() : pkgAcqMethod("1.2",Pipeline | SendConfig), File(NULL) + HttpsMethod() : pkgAcqMethod("1.2",Pipeline | SendConfig), TotalWritten(0), File(NULL) { File = 0; curl = curl_easy_init(); -- cgit v1.2.3 From 62acbba8d1e8c9f395ccd1068e89bf14a93d4aa8 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 7 Oct 2014 08:16:51 +0200 Subject: methods/https.cc: use File->Tell() here too --- methods/https.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'methods') diff --git a/methods/https.cc b/methods/https.cc index eec858417..f8e84a2ff 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -82,8 +82,7 @@ HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp) if(me->File->Write(buffer, size*nmemb) != true) return false; - me->TotalWritten += size*nmemb; - if(me->TotalWritten > me->Queue->ExpectedSize) + if(me->Queue->ExpectedSize > 0 && me->File->Tell() > me->Queue->ExpectedSize) return _error->Error("Writing more data than expected (%llu > %llu)", me->TotalWritten, me->Queue->ExpectedSize); -- cgit v1.2.3 From 5b33fab8c9a8c26cbc8ce3bf5e1573279d7884b3 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 7 Oct 2014 08:43:46 +0200 Subject: add ftp expected size check --- methods/ftp.cc | 10 +++++++--- methods/ftp.h | 2 +- methods/https.cc | 1 - 3 files changed, 8 insertions(+), 5 deletions(-) (limited to 'methods') diff --git a/methods/ftp.cc b/methods/ftp.cc index ac76295f0..75ace1c5a 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -849,7 +849,7 @@ bool FTPConn::Finalize() /* This opens a data connection, sends REST and RETR and then transfers the file over. */ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, - Hashes &Hash,bool &Missing) + Hashes &Hash,bool &Missing, unsigned long long ExpectedSize) { Missing = false; if (CreateDataFd() == false) @@ -922,7 +922,11 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, { Close(); return false; - } + } + + if (ExpectedSize > 0 && To.Tell() > ExpectedSize) + return _error->Error("Writing more data than expected (%llu > %llu)", + To.Tell(), ExpectedSize); } // All done @@ -1063,7 +1067,7 @@ bool FtpMethod::Fetch(FetchItem *Itm) FailFd = Fd.Fd(); bool Missing; - if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing) == false) + if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->ExpectedSize) == false) { Fd.Close(); diff --git a/methods/ftp.h b/methods/ftp.h index dd92f0086..416a91980 100644 --- a/methods/ftp.h +++ b/methods/ftp.h @@ -62,7 +62,7 @@ class FTPConn bool Size(const char *Path,unsigned long long &Size); bool ModTime(const char *Path, time_t &Time); bool Get(const char *Path,FileFd &To,unsigned long long Resume, - Hashes &MD5,bool &Missing); + Hashes &MD5,bool &Missing, unsigned long long ExpectedSize); FTPConn(URI Srv); ~FTPConn(); diff --git a/methods/https.cc b/methods/https.cc index f8e84a2ff..a4794e705 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -86,7 +86,6 @@ HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp) return _error->Error("Writing more data than expected (%llu > %llu)", me->TotalWritten, me->Queue->ExpectedSize); - return size*nmemb; } -- cgit v1.2.3 From c48eea97b93920062ea26001081d4fdf7eb967e3 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 7 Oct 2014 17:47:30 +0200 Subject: make expected-size a maximum-size check as this is what we want at this point --- methods/ftp.cc | 8 ++++---- methods/ftp.h | 2 +- methods/http.cc | 4 ++-- methods/https.cc | 4 ++-- methods/server.cc | 4 ++-- methods/server.h | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) (limited to 'methods') diff --git a/methods/ftp.cc b/methods/ftp.cc index 75ace1c5a..bc84dda7d 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -849,7 +849,7 @@ bool FTPConn::Finalize() /* This opens a data connection, sends REST and RETR and then transfers the file over. */ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, - Hashes &Hash,bool &Missing, unsigned long long ExpectedSize) + Hashes &Hash,bool &Missing, unsigned long long MaximumSize) { Missing = false; if (CreateDataFd() == false) @@ -924,9 +924,9 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, return false; } - if (ExpectedSize > 0 && To.Tell() > ExpectedSize) + if (MaximumSize > 0 && To.Tell() > MaximumSize) return _error->Error("Writing more data than expected (%llu > %llu)", - To.Tell(), ExpectedSize); + To.Tell(), MaximumSize); } // All done @@ -1067,7 +1067,7 @@ bool FtpMethod::Fetch(FetchItem *Itm) FailFd = Fd.Fd(); bool Missing; - if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->ExpectedSize) == false) + if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->MaximumSize) == false) { Fd.Close(); diff --git a/methods/ftp.h b/methods/ftp.h index 416a91980..a31ebc999 100644 --- a/methods/ftp.h +++ b/methods/ftp.h @@ -62,7 +62,7 @@ class FTPConn bool Size(const char *Path,unsigned long long &Size); bool ModTime(const char *Path, time_t &Time); bool Get(const char *Path,FileFd &To,unsigned long long Resume, - Hashes &MD5,bool &Missing, unsigned long long ExpectedSize); + Hashes &MD5,bool &Missing, unsigned long long MaximumSize); FTPConn(URI Srv); ~FTPConn(); diff --git a/methods/http.cc b/methods/http.cc index b076e59cc..f8faa0cf8 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -655,10 +655,10 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File) return _error->Errno("write",_("Error writing to output file")); } - if (ExpectedSize > 0 && File && File->Tell() > ExpectedSize) + if (MaximumSize > 0 && File && File->Tell() > MaximumSize) { return _error->Error("Writing more data than expected (%llu > %llu)", - File->Tell(), ExpectedSize); + File->Tell(), MaximumSize); } // Handle commands from APT diff --git a/methods/https.cc b/methods/https.cc index a4794e705..787e4a507 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -82,9 +82,9 @@ HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp) if(me->File->Write(buffer, size*nmemb) != true) return false; - if(me->Queue->ExpectedSize > 0 && me->File->Tell() > me->Queue->ExpectedSize) + if(me->Queue->MaximumSize > 0 && me->File->Tell() > me->Queue->MaximumSize) return _error->Error("Writing more data than expected (%llu > %llu)", - me->TotalWritten, me->Queue->ExpectedSize); + me->TotalWritten, me->Queue->MaximumSize); return size*nmemb; } diff --git a/methods/server.cc b/methods/server.cc index 223737901..82f9b4750 100644 --- a/methods/server.cc +++ b/methods/server.cc @@ -534,8 +534,8 @@ int ServerMethod::Loop() bool Result = true; // ensure we don't fetch too much - if (Queue->ExpectedSize > 0) - Server->ExpectedSize = Queue->ExpectedSize; + if (Queue->MaximumSize > 0) + Server->MaximumSize = Queue->MaximumSize; if (Server->HaveContent) Result = Server->RunData(File); diff --git a/methods/server.h b/methods/server.h index 0134a9538..3093e00c9 100644 --- a/methods/server.h +++ b/methods/server.h @@ -49,7 +49,7 @@ struct ServerState URI Proxy; unsigned long TimeOut; - unsigned long long ExpectedSize; + unsigned long long MaximumSize; protected: ServerMethod *Owner; @@ -75,7 +75,7 @@ struct ServerState bool Comp(URI Other) const {return Other.Host == ServerName.Host && Other.Port == ServerName.Port;}; virtual void Reset() {Major = 0; Minor = 0; Result = 0; Code[0] = '\0'; Size = 0; StartPos = 0; Encoding = Closes; time(&Date); HaveContent = false; - State = Header; Persistent = false; Pipeline = true; ExpectedSize = 0;}; + State = Header; Persistent = false; Pipeline = true; MaximumSize = 0;}; virtual bool WriteResponse(std::string const &Data) = 0; /** \brief Transfer the data from the socket */ -- cgit v1.2.3 From ee27950632c149bb14c9c490e92147579ba4fc2a Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 7 Oct 2014 22:36:09 +0200 Subject: Send "Fail-Reason: MaximumSizeExceeded" from the method Communicate the fail reason from the methods to the parent and Rename() failed files. --- methods/ftp.cc | 8 ++++++-- methods/ftp.h | 3 ++- methods/http.cc | 1 + methods/https.cc | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) (limited to 'methods') diff --git a/methods/ftp.cc b/methods/ftp.cc index bc84dda7d..5b739ea06 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -849,7 +849,8 @@ bool FTPConn::Finalize() /* This opens a data connection, sends REST and RETR and then transfers the file over. */ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, - Hashes &Hash,bool &Missing, unsigned long long MaximumSize) + Hashes &Hash,bool &Missing, unsigned long long MaximumSize, + pkgAcqMethod *Owner) { Missing = false; if (CreateDataFd() == false) @@ -925,8 +926,11 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume, } if (MaximumSize > 0 && To.Tell() > MaximumSize) + { + Owner->SetFailReason("MaximumSizeExceeded"); return _error->Error("Writing more data than expected (%llu > %llu)", To.Tell(), MaximumSize); + } } // All done @@ -1067,7 +1071,7 @@ bool FtpMethod::Fetch(FetchItem *Itm) FailFd = Fd.Fd(); bool Missing; - if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->MaximumSize) == false) + if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->MaximumSize,this) == false) { Fd.Close(); diff --git a/methods/ftp.h b/methods/ftp.h index a31ebc999..2efd28ec6 100644 --- a/methods/ftp.h +++ b/methods/ftp.h @@ -62,7 +62,8 @@ class FTPConn bool Size(const char *Path,unsigned long long &Size); bool ModTime(const char *Path, time_t &Time); bool Get(const char *Path,FileFd &To,unsigned long long Resume, - Hashes &MD5,bool &Missing, unsigned long long MaximumSize); + Hashes &MD5,bool &Missing, unsigned long long MaximumSize, + pkgAcqMethod *Owner); FTPConn(URI Srv); ~FTPConn(); diff --git a/methods/http.cc b/methods/http.cc index f8faa0cf8..c00b439b7 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -657,6 +657,7 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File) if (MaximumSize > 0 && File && File->Tell() > MaximumSize) { + Owner->SetFailReason("MaximumSizeExceeded"); return _error->Error("Writing more data than expected (%llu > %llu)", File->Tell(), MaximumSize); } diff --git a/methods/https.cc b/methods/https.cc index 787e4a507..16d564b34 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -83,9 +83,11 @@ HttpsMethod::write_data(void *buffer, size_t size, size_t nmemb, void *userp) return false; if(me->Queue->MaximumSize > 0 && me->File->Tell() > me->Queue->MaximumSize) + { + me->SetFailReason("MaximumSizeExceeded"); return _error->Error("Writing more data than expected (%llu > %llu)", me->TotalWritten, me->Queue->MaximumSize); - + } return size*nmemb; } -- cgit v1.2.3 From f2b47ba290f3a178c584da83f007cf0f720baabb Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 Oct 2014 08:32:42 +0200 Subject: Fix http pipeline messup detection The Maximum-Size protection breaks the http pipeline reorder code because it relies on that the object got fetched entirely so that it can compare the hash of the downloaded data. So instead of stopping when the Maximum-Size of the expected item is reached we only stop when the maximum size of the biggest item in the queue is reached. This way the pipeline reoder code keeps working. --- methods/server.cc | 16 ++++++++++++++-- methods/server.h | 4 ++++ 2 files changed, 18 insertions(+), 2 deletions(-) (limited to 'methods') diff --git a/methods/server.cc b/methods/server.cc index 82f9b4750..cef809738 100644 --- a/methods/server.cc +++ b/methods/server.cc @@ -534,8 +534,10 @@ int ServerMethod::Loop() bool Result = true; // ensure we don't fetch too much - if (Queue->MaximumSize > 0) - Server->MaximumSize = Queue->MaximumSize; + // we could do "Server->MaximumSize = Queue->MaximumSize" here + // but that would break the clever pipeline messup detection + // so instead we use the size of the biggest item in the queue + Server->MaximumSize = FindMaximumObjectSizeInQueue(); if (Server->HaveContent) Result = Server->RunData(File); @@ -706,5 +708,15 @@ int ServerMethod::Loop() } return 0; +} + /*}}}*/ + /*{{{*/ +unsigned long long +ServerMethod::FindMaximumObjectSizeInQueue() const +{ + unsigned long long MaxSizeInQueue = 0; + for (FetchItem *I = Queue->Next; I != 0 && I != QueueBack; I = I->Next) + MaxSizeInQueue = std::max(MaxSizeInQueue, I->MaximumSize); + return MaxSizeInQueue; } /*}}}*/ diff --git a/methods/server.h b/methods/server.h index 3093e00c9..7d5198478 100644 --- a/methods/server.h +++ b/methods/server.h @@ -106,6 +106,10 @@ class ServerMethod : public pkgAcqMethod unsigned long PipelineDepth; bool AllowRedirect; + // Find the biggest item in the fetch queue for the checking of the maximum + // size + unsigned long long FindMaximumObjectSizeInQueue() const APT_PURE; + public: bool Debug; -- cgit v1.2.3