From 6862a9b0320d8c07db3a86b20131ab78c5bc4708 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 5 Jul 2016 13:07:29 +0200 Subject: avoid 416 response teardown binding to null pointer methods/http.cc:640:13: runtime error: reference binding to null pointer of type 'struct FileFd' This reference is never used in the cases it has a nullptr, so the practical difference is non-existent, but its a bug still. Reported-By: gcc -fsanitize=undefined (cherry picked from commit 4460551841d909d3ee9c1de00156ed3cdf8b1665) --- methods/http.cc | 16 +++++++++------- methods/http.h | 2 +- methods/https.h | 2 +- methods/server.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-) (limited to 'methods') diff --git a/methods/http.cc b/methods/http.cc index 0c3803fbb..46d90e256 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -498,20 +498,22 @@ APT_PURE Hashes * HttpServerState::GetHashes() /*{{{*/ } /*}}}*/ // HttpServerState::Die - The server has closed the connection. /*{{{*/ -bool HttpServerState::Die(FileFd &File) +bool HttpServerState::Die(FileFd * const File) { unsigned int LErrno = errno; // Dump the buffer to the file if (State == ServerState::Data) { + if (File == nullptr) + return true; // on GNU/kFreeBSD, apt dies on /dev/null because non-blocking // can't be set - if (File.Name() != "/dev/null") - SetNonBlock(File.Fd(),false); + if (File->Name() != "/dev/null") + SetNonBlock(File->Fd(),false); while (In.WriteSpace() == true) { - if (In.Write(File.Fd()) == false) + if (In.Write(File->Fd()) == false) return _error->Errno("write",_("Error writing to the file")); // Done @@ -630,7 +632,7 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File) if (Res == 0) { _error->Error(_("Connection timed out")); - return Die(*File); + return Die(File); } // Handle server IO @@ -638,14 +640,14 @@ bool HttpServerState::Go(bool ToFile, FileFd * const File) { errno = 0; if (In.Read(ServerFd) == false) - return Die(*File); + return Die(File); } if (ServerFd != -1 && FD_ISSET(ServerFd,&wfds)) { errno = 0; if (Out.Write(ServerFd) == false) - return Die(*File); + return Die(File); } // Send data to the file diff --git a/methods/http.h b/methods/http.h index 9e2b1da5c..e3d2a77ff 100644 --- a/methods/http.h +++ b/methods/http.h @@ -113,7 +113,7 @@ struct HttpServerState: public ServerState virtual bool Close() APT_OVERRIDE; virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE; virtual Hashes * GetHashes() APT_OVERRIDE; - virtual bool Die(FileFd &File) APT_OVERRIDE; + virtual bool Die(FileFd * const File) APT_OVERRIDE; virtual bool Flush(FileFd * const File) APT_OVERRIDE; virtual bool Go(bool ToFile, FileFd * const File) APT_OVERRIDE; diff --git a/methods/https.h b/methods/https.h index 4d50c5a04..7d49d9882 100644 --- a/methods/https.h +++ b/methods/https.h @@ -47,7 +47,7 @@ class HttpsServerState : public ServerState virtual bool Close() APT_OVERRIDE { return false; } virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE; virtual Hashes * GetHashes() APT_OVERRIDE; - virtual bool Die(FileFd &/*File*/) APT_OVERRIDE { return false; } + virtual bool Die(FileFd * const /*File*/) APT_OVERRIDE { return false; } virtual bool Flush(FileFd * const /*File*/) APT_OVERRIDE { return false; } virtual bool Go(bool /*ToFile*/, FileFd * const /*File*/) APT_OVERRIDE { return false; } diff --git a/methods/server.h b/methods/server.h index 3f6502432..28c6851f1 100644 --- a/methods/server.h +++ b/methods/server.h @@ -98,7 +98,7 @@ struct ServerState virtual bool Close() = 0; virtual bool InitHashes(HashStringList const &ExpectedHashes) = 0; virtual Hashes * GetHashes() = 0; - virtual bool Die(FileFd &File) = 0; + virtual bool Die(FileFd * const File) = 0; virtual bool Flush(FileFd * const File) = 0; virtual bool Go(bool ToFile, FileFd * const File) = 0; -- cgit v1.2.3